)]}'
{"nova/tests/unit/virt/libvirt/test_imagecache.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d5340f1e9db27bd39595c141f625fc7f20683d7","unresolved":false,"context_lines":[{"line_number":463,"context_line":"                             [fname])"},{"line_number":464,"context_line":"            self.assertEqual(image_cache_manager.corrupt_base_files, [])"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    @mock.patch.object(libvirt_utils, \u0027update_mtime\u0027)"},{"line_number":467,"context_line":"    def test_handle_base_image_used(self, mock_mtime):"},{"line_number":468,"context_line":"        img \u003d \u0027123\u0027"},{"line_number":469,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_07580155","line":466,"updated":"2015-12-08 15:31:53.000000000","message":"How about verifying that this is actually called when we expect?","commit_id":"871cc4b69e34eacb733e897be9187df9d1f65e38"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d5340f1e9db27bd39595c141f625fc7f20683d7","unresolved":false,"context_lines":[{"line_number":896,"context_line":"        image_cache_manager \u003d imagecache.ImageCacheManager()"},{"line_number":897,"context_line":"        image_cache_manager._remove_old_enough_file("},{"line_number":898,"context_line":"            \u0027/tmp_age_test\u0027, 60, remove_sig\u003dFalse, remove_lock\u003dFalse)"},{"line_number":899,"context_line":"        self.assertTrue(mock_synchronized.called)"},{"line_number":900,"context_line":""},{"line_number":901,"context_line":""},{"line_number":902,"context_line":"class VerifyChecksumTestCase(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_c202e709","line":899,"updated":"2015-12-08 15:31:53.000000000","message":"Can\u0027t you check that this was called with more detail? This is a really risky thing to use as the pass/fail for the test. Any call to synchronized in this path will cause this test to pass.","commit_id":"871cc4b69e34eacb733e897be9187df9d1f65e38"}],"nova/virt/libvirt/imagebackend.py":[{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"31cf8d94194f0437ddd32ce5c8e682ee8e791ed9","unresolved":false,"context_lines":[{"line_number":448,"context_line":"                # Generating image in place"},{"line_number":449,"context_line":"                prepare_template(target\u003dself.path, *args, **kwargs)"},{"line_number":450,"context_line":"        else:"},{"line_number":451,"context_line":"            if not os.path.exists(base):"},{"line_number":452,"context_line":"                prepare_template(target\u003dbase, max_size\u003dsize, *args, **kwargs)"},{"line_number":453,"context_line":"            self.verify_base_size(base, size)"},{"line_number":454,"context_line":"            if not os.path.exists(self.path):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_c7983f6d","line":451,"updated":"2015-08-10 23:17:35.000000000","message":"I feel like we should update the utime on the base file here, which means we shouldn\u0027t have to guard against a retry below, right?","commit_id":"a1c041dd0269f305ad025cdc3516ba14e99f01ba"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"f6d850b9e71be1541895a0a73b2a256064dc1ff3","unresolved":false,"context_lines":[{"line_number":448,"context_line":"                # Generating image in place"},{"line_number":449,"context_line":"                prepare_template(target\u003dself.path, *args, **kwargs)"},{"line_number":450,"context_line":"        else:"},{"line_number":451,"context_line":"            if not os.path.exists(base):"},{"line_number":452,"context_line":"                prepare_template(target\u003dbase, max_size\u003dsize, *args, **kwargs)"},{"line_number":453,"context_line":"            self.verify_base_size(base, size)"},{"line_number":454,"context_line":"            if not os.path.exists(self.path):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_0b328d5e","line":451,"in_reply_to":"1a4dcd0f_8f60f736","updated":"2015-08-11 13:46:40.000000000","message":"I have submitted a new PS with rootwrap changes but the granade tests will fail on Jenkins for this change.\n\nAlternatively can we propose the changes uploaded in test patch Ia7629de15055c40523882fa2fbc808e3cfe6fa6d with updating the retry logic as you added in PS-4.","commit_id":"a1c041dd0269f305ad025cdc3516ba14e99f01ba"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"1b12e39d69200ff0e4192a134016d610def42611","unresolved":false,"context_lines":[{"line_number":448,"context_line":"                # Generating image in place"},{"line_number":449,"context_line":"                prepare_template(target\u003dself.path, *args, **kwargs)"},{"line_number":450,"context_line":"        else:"},{"line_number":451,"context_line":"            if not os.path.exists(base):"},{"line_number":452,"context_line":"                prepare_template(target\u003dbase, max_size\u003dsize, *args, **kwargs)"},{"line_number":453,"context_line":"            self.verify_base_size(base, size)"},{"line_number":454,"context_line":"            if not os.path.exists(self.path):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_8f60f736","line":451,"in_reply_to":"1a4dcd0f_c7983f6d","updated":"2015-08-11 10:34:55.000000000","message":"Yes we can update utime here and the retry logic similar to your new patch set. But In your new patch set you have used \"touch\" which will fail because \"touch\" is not there in rootwrap filters.","commit_id":"a1c041dd0269f305ad025cdc3516ba14e99f01ba"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"31cf8d94194f0437ddd32ce5c8e682ee8e791ed9","unresolved":false,"context_lines":[{"line_number":515,"context_line":"            # to age of the file."},{"line_number":516,"context_line":"            libvirt_utils.chown(base, os.getuid())"},{"line_number":517,"context_line":"            # Wait for subprocess to finish updating the permissions."},{"line_number":518,"context_line":"            time.sleep(0.5)"},{"line_number":519,"context_line":"            os.utime(base, None)"},{"line_number":520,"context_line":"            if size:"},{"line_number":521,"context_line":"                image \u003d imgmodel.LocalFileImage(target, imgmodel.FORMAT_QCOW2)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_27d6c327","line":518,"range":{"start_line":518,"start_character":16,"end_line":518,"end_character":17},"updated":"2015-08-10 23:17:35.000000000","message":"I don\u0027t think this sleep is a good idea, and I\u0027d like to explore why it is necessary. The chown should not return until the command has executed, right?","commit_id":"a1c041dd0269f305ad025cdc3516ba14e99f01ba"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"1b12e39d69200ff0e4192a134016d610def42611","unresolved":false,"context_lines":[{"line_number":515,"context_line":"            # to age of the file."},{"line_number":516,"context_line":"            libvirt_utils.chown(base, os.getuid())"},{"line_number":517,"context_line":"            # Wait for subprocess to finish updating the permissions."},{"line_number":518,"context_line":"            time.sleep(0.5)"},{"line_number":519,"context_line":"            os.utime(base, None)"},{"line_number":520,"context_line":"            if size:"},{"line_number":521,"context_line":"                image \u003d imgmodel.LocalFileImage(target, imgmodel.FORMAT_QCOW2)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_4ca51d02","line":518,"in_reply_to":"1a4dcd0f_27d6c327","updated":"2015-08-11 10:34:55.000000000","message":"The sleep was added for testing only and is not useful here.","commit_id":"a1c041dd0269f305ad025cdc3516ba14e99f01ba"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"31cf8d94194f0437ddd32ce5c8e682ee8e791ed9","unresolved":false,"context_lines":[{"line_number":516,"context_line":"            libvirt_utils.chown(base, os.getuid())"},{"line_number":517,"context_line":"            # Wait for subprocess to finish updating the permissions."},{"line_number":518,"context_line":"            time.sleep(0.5)"},{"line_number":519,"context_line":"            os.utime(base, None)"},{"line_number":520,"context_line":"            if size:"},{"line_number":521,"context_line":"                image \u003d imgmodel.LocalFileImage(target, imgmodel.FORMAT_QCOW2)"},{"line_number":522,"context_line":"                disk.extend(image, size)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_87e79711","line":519,"range":{"start_line":519,"start_character":12,"end_line":519,"end_character":14},"updated":"2015-08-10 23:17:35.000000000","message":"I am also seeing permissions problems on this line in the gate. For example:\n\n 2015-08-10 20:49:42.090 ERROR nova.compute.manager [req-2d5323e7-cf51-42b5-b72e-3c1699eb74c7 tempest-ServersTestJSON-814538471 tempest-ServersTestJSON-1502647296] [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f] Instance failed to spawn\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f] Traceback (most recent call last):\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]   File  \"/opt/stack/new/nova/nova/compute/manager.py\", line 2114, in _build_resources\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]     yield resources\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]   File   \"/opt/stack/new/nova/nova/compute/manager.py\", line 1986, in _build_and_run_instance\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]     block_device_info\u003dblock_device_info)\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]   File  \"/opt/stack/new/nova/nova/virt/libvirt/driver.py\", line 2431, in spawn\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]     admin_pass\u003dadmin_password)\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]   File  \"/opt/stack/new/nova/nova/virt/libvirt/driver.py\", line 2854, in _create_image\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]     instance, size, fallback_from_host)\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]   File \"/opt/stack/new/nova/nova/virt/libvirt/driver.py\", line 6029, in _try_fetch_image_cache\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]     size\u003dsize)\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]   File \"/opt/stack/new/nova/nova/virt/libvirt/imagebackend.py\", line 238, in cache\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]     *args, **kwargs)\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]   File \"/opt/stack/new/nova/nova/virt/libvirt/imagebackend.py\", line 575, in create_image\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]     copy_qcow2_image(base, self.path, size)\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]   File \"/usr/local/lib/python2.7/dist-packages/oslo_concurrency/lockutils.py\", line 252, in inner\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]     return f(*args, **kwargs)\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]   File \"/opt/stack/new/nova/nova/virt/libvirt/imagebackend.py\", line 536, in copy_qcow2_image\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]     os.utime(base, None)\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f] OSError: [Errno 13] Permission denied: \u0027/opt/stack/data/nova/instances/_base/a832d16ed29b880888aca59de851664be4dc07b2\u0027\n 2015-08-10 20:49:42.090 4072 ERROR nova.compute.manager [instance: 7d0fac88-0431-49e8-b85b-1e1382946d1f]","commit_id":"a1c041dd0269f305ad025cdc3516ba14e99f01ba"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"1b12e39d69200ff0e4192a134016d610def42611","unresolved":false,"context_lines":[{"line_number":516,"context_line":"            libvirt_utils.chown(base, os.getuid())"},{"line_number":517,"context_line":"            # Wait for subprocess to finish updating the permissions."},{"line_number":518,"context_line":"            time.sleep(0.5)"},{"line_number":519,"context_line":"            os.utime(base, None)"},{"line_number":520,"context_line":"            if size:"},{"line_number":521,"context_line":"                image \u003d imgmodel.LocalFileImage(target, imgmodel.FORMAT_QCOW2)"},{"line_number":522,"context_line":"                disk.extend(image, size)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_7271d0a7","line":519,"in_reply_to":"1a4dcd0f_87e79711","updated":"2015-08-11 10:34:55.000000000","message":"This permission denied error is raised in a race condition when file ownership and last access time is changed by guest.launch method from libvirt driver just after executing the line #516 and before line #519.\n\nAs last access time has already bean updated by guest.launch method when ownership is changed so we can simply log a message and pass this exception.\n\nguest.launch method updates the last access time but not the last modified time of the file so I have changed the logic in imagecache manger to calculate the age of file.\nI have submitted a test patch which passes all the tests successfully on jenkins Ia7629de15055c40523882fa2fbc808e3cfe6fa6d","commit_id":"a1c041dd0269f305ad025cdc3516ba14e99f01ba"},{"author":{"_account_id":14358,"name":"Marian Horban","email":"m.horban@gmail.com","username":"mhorban"},"change_message_id":"ada6e24e04ef6e8f1e23d4bc04d9754e69a0c7c6","unresolved":false,"context_lines":[{"line_number":479,"context_line":""},{"line_number":480,"context_line":"            attempts \u003d 0"},{"line_number":481,"context_line":"            while not os.path.exists(self.path):"},{"line_number":482,"context_line":"                if attempts \u003e 2:"},{"line_number":483,"context_line":"                    raise exception.ImageCopyFailed("},{"line_number":484,"context_line":"                        reason\u003d_(\u0027too many attempts\u0027))"},{"line_number":485,"context_line":"                attempts +\u003d 1"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_ea267367","line":482,"updated":"2015-08-27 16:55:57.000000000","message":"Why exactly 3 tries is allowed?","commit_id":"ac12f82f765a7e7e50d611c684ac9387f9547e41"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"ef7735dbecbaf4441b9ecadef210ad31cadc660f","unresolved":false,"context_lines":[{"line_number":479,"context_line":""},{"line_number":480,"context_line":"            attempts \u003d 0"},{"line_number":481,"context_line":"            while not os.path.exists(self.path):"},{"line_number":482,"context_line":"                if attempts \u003e 2:"},{"line_number":483,"context_line":"                    raise exception.ImageCopyFailed("},{"line_number":484,"context_line":"                        reason\u003d_(\u0027too many attempts\u0027))"},{"line_number":485,"context_line":"                attempts +\u003d 1"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_51592afd","line":482,"in_reply_to":"da20952f_ea267367","updated":"2015-08-28 12:39:51.000000000","message":"I\u0027ll make it configurable. Thanks","commit_id":"ac12f82f765a7e7e50d611c684ac9387f9547e41"},{"author":{"_account_id":14358,"name":"Marian Horban","email":"m.horban@gmail.com","username":"mhorban"},"change_message_id":"ada6e24e04ef6e8f1e23d4bc04d9754e69a0c7c6","unresolved":false,"context_lines":[{"line_number":492,"context_line":"                    # while we were setting up the raw image. We therefore"},{"line_number":493,"context_line":"                    # retry."},{"line_number":494,"context_line":"                    if attempts \u003e 2:"},{"line_number":495,"context_line":"                        raise exc"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":"        self.correct_format()"},{"line_number":498,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_6ac90378","line":495,"updated":"2015-08-27 16:55:57.000000000","message":"I believe that ImageCopyFailed should be raised here","commit_id":"ac12f82f765a7e7e50d611c684ac9387f9547e41"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"bff49a8e1b96891bacf4a0da7ff8d3a9d78e37c7","unresolved":false,"context_lines":[{"line_number":501,"context_line":"                    break"},{"line_number":502,"context_line":"                except processutils.ProcessExecutionError:"},{"line_number":503,"context_line":"                    # copy_raw_image could fail if the base file disappears,"},{"line_number":504,"context_line":"                    # therefore keeping retrying max_retry times."},{"line_number":505,"context_line":"                    attempts +\u003d 1"},{"line_number":506,"context_line":"                    if attempts \u003e max_retry:"},{"line_number":507,"context_line":"                        raise exception.ImageCopyFailed("}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_abb093f5","line":504,"updated":"2015-09-25 15:24:25.000000000","message":"Why would the base file disappear now, given the extra synchronization in place?\nIdeally we\u0027d not have a new config item for this. If this is really needed then it would be good to have a better description in the help\u003d item above to give the user some indication as to how/why they might adjust the config option.","commit_id":"cd9382549c8409ac64c9f5acad1a26996055e730"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"3a766633f7719652b28816ed52e04f5a8965bcf0","unresolved":false,"context_lines":[{"line_number":501,"context_line":"                    break"},{"line_number":502,"context_line":"                except processutils.ProcessExecutionError:"},{"line_number":503,"context_line":"                    # copy_raw_image could fail if the base file disappears,"},{"line_number":504,"context_line":"                    # therefore keeping retrying max_retry times."},{"line_number":505,"context_line":"                    attempts +\u003d 1"},{"line_number":506,"context_line":"                    if attempts \u003e max_retry:"},{"line_number":507,"context_line":"                        raise exception.ImageCopyFailed("}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_d038dea2","line":504,"in_reply_to":"9a1a9d01_abb093f5","updated":"2015-09-28 12:18:20.000000000","message":"This situation may occur if base file is deleted from image cache while waiting to acquire lock in the copy_raw_image method.","commit_id":"cd9382549c8409ac64c9f5acad1a26996055e730"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"89ce4e23f9e24289eea3a2489966c1a02c95a6f2","unresolved":false,"context_lines":[{"line_number":501,"context_line":"                    break"},{"line_number":502,"context_line":"                except processutils.ProcessExecutionError:"},{"line_number":503,"context_line":"                    # copy_raw_image could fail if the base file disappears,"},{"line_number":504,"context_line":"                    # therefore keeping retrying max_retry times."},{"line_number":505,"context_line":"                    attempts +\u003d 1"},{"line_number":506,"context_line":"                    if attempts \u003e max_retry:"},{"line_number":507,"context_line":"                        raise exception.ImageCopyFailed("}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_95a43cba","line":504,"in_reply_to":"9a1a9d01_abb093f5","updated":"2015-09-30 06:56:10.000000000","message":"p-draigbrady, After updating the last access time at line #491 the bace file will not be deleted by imagecache manager, so we don\u0027t need to retry now. Thanks.","commit_id":"cd9382549c8409ac64c9f5acad1a26996055e730"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"369baca0e7ae9f281a0b5c200a6631003e775278","unresolved":false,"context_lines":[{"line_number":593,"context_line":"                # therefore keeping retrying max_retry times."},{"line_number":594,"context_line":"                attempts +\u003d 1"},{"line_number":595,"context_line":"                if attempts \u003e max_retry:"},{"line_number":596,"context_line":"                    raise"},{"line_number":597,"context_line":""},{"line_number":598,"context_line":"    def snapshot_extract(self, target, out_format):"},{"line_number":599,"context_line":"        libvirt_utils.extract_snapshot(self.path, \u0027qcow2\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_49809846","line":596,"updated":"2015-09-17 10:36:54.000000000","message":"Nit not liking the duplicate code with above, but don\u0027t have any big ideas, given the slight differences.","commit_id":"cd9382549c8409ac64c9f5acad1a26996055e730"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"80e166da6d7b2d9638b03a3dad20952ffe4258e2","unresolved":false,"context_lines":[{"line_number":593,"context_line":"                # therefore keeping retrying max_retry times."},{"line_number":594,"context_line":"                attempts +\u003d 1"},{"line_number":595,"context_line":"                if attempts \u003e max_retry:"},{"line_number":596,"context_line":"                    raise"},{"line_number":597,"context_line":""},{"line_number":598,"context_line":"    def snapshot_extract(self, target, out_format):"},{"line_number":599,"context_line":"        libvirt_utils.extract_snapshot(self.path, \u0027qcow2\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_cff6858e","line":596,"in_reply_to":"ba15a1d1_49809846","updated":"2015-09-18 06:19:23.000000000","message":"Hi John,\n\nHere we are calling copy_qcow2_image method which raises DiskNotFound exception in case of failure, whereas in above case we are calling copy_raw_image which raises ProcessExecutionError. Because of these differences I kept it separate.","commit_id":"cd9382549c8409ac64c9f5acad1a26996055e730"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"36d41d2efb94cafac181c83db99318822eabaf34","unresolved":false,"context_lines":[{"line_number":593,"context_line":"                # therefore keeping retrying max_retry times."},{"line_number":594,"context_line":"                attempts +\u003d 1"},{"line_number":595,"context_line":"                if attempts \u003e max_retry:"},{"line_number":596,"context_line":"                    raise"},{"line_number":597,"context_line":""},{"line_number":598,"context_line":"    def snapshot_extract(self, target, out_format):"},{"line_number":599,"context_line":"        libvirt_utils.extract_snapshot(self.path, \u0027qcow2\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_283d732b","line":596,"in_reply_to":"ba15a1d1_cff6858e","updated":"2015-09-25 10:30:44.000000000","message":"yes, I think thats the best option, I was just thinking out loud really.","commit_id":"cd9382549c8409ac64c9f5acad1a26996055e730"}],"nova/virt/libvirt/imagecache.py":[{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"31cf8d94194f0437ddd32ce5c8e682ee8e791ed9","unresolved":false,"context_lines":[{"line_number":459,"context_line":"            @utils.synchronized(lock_file, external\u003dTrue,"},{"line_number":460,"context_line":"                                lock_path\u003dself.lock_path)"},{"line_number":461,"context_line":"            def inner_remove_old_enough_file():"},{"line_number":462,"context_line":"                LOG.info(_LI(\u0027Removing base or swap file: %s\u0027), base_file)"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"                try:"},{"line_number":465,"context_line":"                    os.remove(base_file)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_47ad0f87","line":462,"updated":"2015-08-10 23:17:35.000000000","message":"We should recheck the utime of the file here, in case its been used since we iterated across all the files.","commit_id":"a1c041dd0269f305ad025cdc3516ba14e99f01ba"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"1b12e39d69200ff0e4192a134016d610def42611","unresolved":false,"context_lines":[{"line_number":459,"context_line":"            @utils.synchronized(lock_file, external\u003dTrue,"},{"line_number":460,"context_line":"                                lock_path\u003dself.lock_path)"},{"line_number":461,"context_line":"            def inner_remove_old_enough_file():"},{"line_number":462,"context_line":"                LOG.info(_LI(\u0027Removing base or swap file: %s\u0027), base_file)"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"                try:"},{"line_number":465,"context_line":"                    os.remove(base_file)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_2c3691e7","line":462,"in_reply_to":"1a4dcd0f_47ad0f87","updated":"2015-08-11 10:34:55.000000000","message":"Added this recheck in my test patch Ia7629de15055c40523882fa2fbc808e3cfe6fa6d","commit_id":"a1c041dd0269f305ad025cdc3516ba14e99f01ba"},{"author":{"_account_id":14358,"name":"Marian Horban","email":"m.horban@gmail.com","username":"mhorban"},"change_message_id":"ada6e24e04ef6e8f1e23d4bc04d9754e69a0c7c6","unresolved":false,"context_lines":[{"line_number":478,"context_line":"                            os.remove(signature)"},{"line_number":479,"context_line":"                except OSError as e:"},{"line_number":480,"context_line":"                    LOG.error(_LE(\u0027Failed to remove %(base_file)s, \u0027"},{"line_number":481,"context_line":"                                  \u0027error was %(error)s\u0027),"},{"line_number":482,"context_line":"                              {\u0027base_file\u0027: base_file,"},{"line_number":483,"context_line":"                               \u0027error\u0027: e})"},{"line_number":484,"context_line":"            inner_remove_old_enough_file()"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_0aec078e","line":481,"updated":"2015-08-27 16:55:57.000000000","message":"I think it is not appropriate message. Exception could be caused by signature removing as well","commit_id":"ac12f82f765a7e7e50d611c684ac9387f9547e41"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"ef7735dbecbaf4441b9ecadef210ad31cadc660f","unresolved":false,"context_lines":[{"line_number":478,"context_line":"                            os.remove(signature)"},{"line_number":479,"context_line":"                except OSError as e:"},{"line_number":480,"context_line":"                    LOG.error(_LE(\u0027Failed to remove %(base_file)s, \u0027"},{"line_number":481,"context_line":"                                  \u0027error was %(error)s\u0027),"},{"line_number":482,"context_line":"                              {\u0027base_file\u0027: base_file,"},{"line_number":483,"context_line":"                               \u0027error\u0027: e})"},{"line_number":484,"context_line":"            inner_remove_old_enough_file()"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_d191ba9a","line":481,"in_reply_to":"da20952f_0aec078e","updated":"2015-08-28 12:39:51.000000000","message":"I have just moved this code block in to a inner function as it is. If we need to modify the log message, it can be handled in separate patch IMHO as this change is not related to the fix.","commit_id":"ac12f82f765a7e7e50d611c684ac9387f9547e41"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"bff49a8e1b96891bacf4a0da7ff8d3a9d78e37c7","unresolved":false,"context_lines":[{"line_number":563,"context_line":"                          {\u0027id\u0027: img_id,"},{"line_number":564,"context_line":"                           \u0027base_file\u0027: base_file})"},{"line_number":565,"context_line":"                if os.path.exists(base_file):"},{"line_number":566,"context_line":"                    libvirt_utils.chown(base_file, os.getuid())"},{"line_number":567,"context_line":"                    os.utime(base_file, None)"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":"    def _age_and_verify_swap_images(self, context, base_dir):"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_4bb1b78e","side":"PARENT","line":566,"updated":"2015-09-25 15:24:25.000000000","message":"The chown() was done to allow os.utime() to _write_ to the image as per bug 973377. Hopefully there have been no other writes from nova to the image in the meantime","commit_id":"279c2b0a015b67c5195ab028eeab15b00a94e5ee"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"369baca0e7ae9f281a0b5c200a6631003e775278","unresolved":false,"context_lines":[{"line_number":563,"context_line":"                          {\u0027id\u0027: img_id,"},{"line_number":564,"context_line":"                           \u0027base_file\u0027: base_file})"},{"line_number":565,"context_line":"                if os.path.exists(base_file):"},{"line_number":566,"context_line":"                    libvirt_utils.chown(base_file, os.getuid())"},{"line_number":567,"context_line":"                    os.utime(base_file, None)"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":"    def _age_and_verify_swap_images(self, context, base_dir):"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_096f0093","side":"PARENT","line":566,"updated":"2015-09-17 10:36:54.000000000","message":"lost the chown call here, is that a good idea? I think I remember bugs in the past when not doing this?","commit_id":"279c2b0a015b67c5195ab028eeab15b00a94e5ee"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"80e166da6d7b2d9638b03a3dad20952ffe4258e2","unresolved":false,"context_lines":[{"line_number":563,"context_line":"                          {\u0027id\u0027: img_id,"},{"line_number":564,"context_line":"                           \u0027base_file\u0027: base_file})"},{"line_number":565,"context_line":"                if os.path.exists(base_file):"},{"line_number":566,"context_line":"                    libvirt_utils.chown(base_file, os.getuid())"},{"line_number":567,"context_line":"                    os.utime(base_file, None)"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":"    def _age_and_verify_swap_images(self, context, base_dir):"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_b23db4c5","side":"PARENT","line":566,"in_reply_to":"ba15a1d1_096f0093","updated":"2015-09-18 06:19:23.000000000","message":"Now we are updating the access time using \u0027touch\u0027 command with root user privileges which does not require to call chown.\n\nPreviously we\u0027re using os.utime which required to call chown in order to provide write access to the base file with current user. Else os.utime raises permission error if base file does not have write access with current user.","commit_id":"279c2b0a015b67c5195ab028eeab15b00a94e5ee"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"36d41d2efb94cafac181c83db99318822eabaf34","unresolved":false,"context_lines":[{"line_number":563,"context_line":"                          {\u0027id\u0027: img_id,"},{"line_number":564,"context_line":"                           \u0027base_file\u0027: base_file})"},{"line_number":565,"context_line":"                if os.path.exists(base_file):"},{"line_number":566,"context_line":"                    libvirt_utils.chown(base_file, os.getuid())"},{"line_number":567,"context_line":"                    os.utime(base_file, None)"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":"    def _age_and_verify_swap_images(self, context, base_dir):"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_882b5feb","side":"PARENT","line":566,"in_reply_to":"ba15a1d1_b23db4c5","updated":"2015-09-25 10:30:44.000000000","message":"OK, but you need the correct user on the file when we want to read from it, I want to make sure this patch does not loose that.","commit_id":"279c2b0a015b67c5195ab028eeab15b00a94e5ee"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"c6fe28e81fa6285f88ea96163750e69c309fa424","unresolved":false,"context_lines":[{"line_number":458,"context_line":""},{"line_number":459,"context_line":"            @utils.synchronized(lock_file, external\u003dTrue,"},{"line_number":460,"context_line":"                                lock_path\u003dself.lock_path)"},{"line_number":461,"context_line":"            def inner_remove_old_enough_file():"},{"line_number":462,"context_line":"                # NOTE(mikal): recheck that the file is old enough, as a new"},{"line_number":463,"context_line":"                # user of the file might have come along while we were waiting"},{"line_number":464,"context_line":"                # for the lock"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a1a9d01_c9f42af2","line":461,"updated":"2015-10-01 08:52:01.000000000","message":"consider using _inner_remove_old_enough_file","commit_id":"871cc4b69e34eacb733e897be9187df9d1f65e38"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d5340f1e9db27bd39595c141f625fc7f20683d7","unresolved":false,"context_lines":[{"line_number":458,"context_line":""},{"line_number":459,"context_line":"            @utils.synchronized(lock_file, external\u003dTrue,"},{"line_number":460,"context_line":"                                lock_path\u003dself.lock_path)"},{"line_number":461,"context_line":"            def inner_remove_old_enough_file():"},{"line_number":462,"context_line":"                # NOTE(mikal): recheck that the file is old enough, as a new"},{"line_number":463,"context_line":"                # user of the file might have come along while we were waiting"},{"line_number":464,"context_line":"                # for the lock"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_27497db7","line":461,"in_reply_to":"5a2ca52d_6d9070ed","updated":"2015-12-08 15:31:53.000000000","message":"I\u0027m fine with the name, but I think defining this inside the else is pretty crazy. Can you define this at the method level and call it from the else, if necessary? i.e. move it to line 452.","commit_id":"871cc4b69e34eacb733e897be9187df9d1f65e38"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"622584bfe6d531d5a88612818c38d16a7bf3cfc6","unresolved":false,"context_lines":[{"line_number":458,"context_line":""},{"line_number":459,"context_line":"            @utils.synchronized(lock_file, external\u003dTrue,"},{"line_number":460,"context_line":"                                lock_path\u003dself.lock_path)"},{"line_number":461,"context_line":"            def inner_remove_old_enough_file():"},{"line_number":462,"context_line":"                # NOTE(mikal): recheck that the file is old enough, as a new"},{"line_number":463,"context_line":"                # user of the file might have come along while we were waiting"},{"line_number":464,"context_line":"                # for the lock"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a2ca52d_6d9070ed","line":461,"in_reply_to":"9a1a9d01_c9f42af2","updated":"2015-10-13 13:38:05.000000000","message":"I\u0027ll update the method name if I needed to submit a new PS, Thanks.","commit_id":"871cc4b69e34eacb733e897be9187df9d1f65e38"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d5340f1e9db27bd39595c141f625fc7f20683d7","unresolved":false,"context_lines":[{"line_number":480,"context_line":"                    LOG.error(_LE(\u0027Failed to remove %(base_file)s, \u0027"},{"line_number":481,"context_line":"                                  \u0027error was %(error)s\u0027),"},{"line_number":482,"context_line":"                              {\u0027base_file\u0027: base_file,"},{"line_number":483,"context_line":"                               \u0027error\u0027: e})"},{"line_number":484,"context_line":"            inner_remove_old_enough_file()"},{"line_number":485,"context_line":"            if remove_lock:"},{"line_number":486,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_278e5dd0","line":483,"updated":"2015-12-08 15:31:53.000000000","message":"There should be one blank line after the function definition here for clarity.","commit_id":"871cc4b69e34eacb733e897be9187df9d1f65e38"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d5340f1e9db27bd39595c141f625fc7f20683d7","unresolved":false,"context_lines":[{"line_number":485,"context_line":"            if remove_lock:"},{"line_number":486,"context_line":"                try:"},{"line_number":487,"context_line":"                    # NOTE(jichenjc) The lock file will be constructed first"},{"line_number":488,"context_line":"                    # time the image file was accessed. The lock file looks"},{"line_number":489,"context_line":"                    # like nova-9e881789030568a317fad9daae82c5b1c65e0d4a"},{"line_number":490,"context_line":"                    # or nova-03d8e206-6500-4d91-b47d-ee74897f9b4e"},{"line_number":491,"context_line":"                    # according to the original file name"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_a76fadfa","line":488,"updated":"2015-12-08 15:31:53.000000000","message":"Changing this comment just adds to the noise in this patch and is unnecessary. Please remove that change.","commit_id":"871cc4b69e34eacb733e897be9187df9d1f65e38"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"b0cbb4230221af753d3c16de5b56c47fdfebe691","unresolved":false,"context_lines":[{"line_number":460,"context_line":"            exists, age \u003d self._get_age_of_file(base_file)"},{"line_number":461,"context_line":"            if not exists:"},{"line_number":462,"context_line":"                return"},{"line_number":463,"context_line":"            if age \u003c maxage:"},{"line_number":464,"context_line":"                return"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"            LOG.info(_LI(\u0027Removing base or swap file: %s\u0027), base_file)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5a5ae5dd_6579ddc7","line":463,"updated":"2016-02-05 19:21:18.000000000","message":"femtonit: could combine these two \u0027if\u0027s into one condition…","commit_id":"9cd0bcfb930865ae246b79a4d255495692294b6c"}],"nova/virt/libvirt/utils.py":[{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"bff49a8e1b96891bacf4a0da7ff8d3a9d78e37c7","unresolved":false,"context_lines":[{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    :param path: File bump the mtime on"},{"line_number":247,"context_line":"    \"\"\""},{"line_number":248,"context_line":"    execute(\u0027touch\u0027, path, run_as_root\u003dTrue)"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"def _id_map_to_config(id_map):"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_abf893e6","line":248,"updated":"2015-09-25 15:24:25.000000000","message":"Do we want to create a file (with root perms) when missing?\nI.E. Do we need -c (--no-create) here?","commit_id":"cd9382549c8409ac64c9f5acad1a26996055e730"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"3a766633f7719652b28816ed52e04f5a8965bcf0","unresolved":false,"context_lines":[{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    :param path: File bump the mtime on"},{"line_number":247,"context_line":"    \"\"\""},{"line_number":248,"context_line":"    execute(\u0027touch\u0027, path, run_as_root\u003dTrue)"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"def _id_map_to_config(id_map):"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a1a9d01_f08c0237","line":248,"in_reply_to":"9a1a9d01_abf893e6","updated":"2015-09-28 12:18:20.000000000","message":"In current code this method is only called if base file exist. But as we don\u0027t need to create a new one when missing so we should definitly add \"-c\" option here. Thanks","commit_id":"cd9382549c8409ac64c9f5acad1a26996055e730"}]}