)]}'
{"cinder/tests/unit/volume/drivers/test_rbd.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6a38d12498fd80a774cc0e406b61ce9286bd1196","unresolved":false,"context_lines":[{"line_number":2486,"context_line":"        ])"},{"line_number":2487,"context_line":"        self.assertEqual(4, image.update_features.call_count)"},{"line_number":2488,"context_line":"        self.assertEqual({\u0027provider_location\u0027: \"{\\\"saved_features\\\":125}\"},"},{"line_number":2489,"context_line":"                         ret)"},{"line_number":2490,"context_line":""},{"line_number":2491,"context_line":"    @common_mocks"},{"line_number":2492,"context_line":"    def test_disable_multiattach(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_f1c5f290","line":2489,"updated":"2020-01-07 22:06:59.000000000","message":"If i understand the bug correctly, the problem is that you get an InvalidArgument exception when you call image.update(key, val) when key already has value val.  This test makes sure that if you start with everything \"on\", the correct ones are turned \"off\".\n\nI think in addition to this test, you may want to add one that has journaling, fast diff, object map, and exlusive lock already \"off\" and make sure _enable_multiattach() doesn\u0027t explode and that they\u0027re still \"off\" after the call.  (Maybe mock image.update_features() to always raise InvalidArgument?)","commit_id":"3e057db79bd95ba53671f92c74df79c4a08e791c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"5a4fdb57ab123ab95dac5acab46c2563be4969b8","unresolved":false,"context_lines":[{"line_number":2486,"context_line":"        ])"},{"line_number":2487,"context_line":"        self.assertEqual(4, image.update_features.call_count)"},{"line_number":2488,"context_line":"        self.assertEqual({\u0027provider_location\u0027: \"{\\\"saved_features\\\":125}\"},"},{"line_number":2489,"context_line":"                         ret)"},{"line_number":2490,"context_line":""},{"line_number":2491,"context_line":"    @common_mocks"},{"line_number":2492,"context_line":"    def test_disable_multiattach(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_0ec8a51e","line":2489,"in_reply_to":"3fa7e38b_f1c5f290","updated":"2020-01-08 14:16:38.000000000","message":"Done","commit_id":"3e057db79bd95ba53671f92c74df79c4a08e791c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6a38d12498fd80a774cc0e406b61ce9286bd1196","unresolved":false,"context_lines":[{"line_number":2502,"context_line":"            call(RBD_FEATURE_FAST_DIFF, True),"},{"line_number":2503,"context_line":"            call(RBD_FEATURE_JOURNALING, True)])"},{"line_number":2504,"context_line":"        self.assertEqual(4, image.update_features.call_count)"},{"line_number":2505,"context_line":"        self.assertEqual({\u0027provider_location\u0027: None}, ret)"},{"line_number":2506,"context_line":""},{"line_number":2507,"context_line":""},{"line_number":2508,"context_line":"class ManagedRBDTestCase(test_driver.BaseDriverTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_71b90214","line":2505,"updated":"2020-01-07 22:06:59.000000000","message":"Similar issue with this test.","commit_id":"3e057db79bd95ba53671f92c74df79c4a08e791c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"5a4fdb57ab123ab95dac5acab46c2563be4969b8","unresolved":false,"context_lines":[{"line_number":2502,"context_line":"            call(RBD_FEATURE_FAST_DIFF, True),"},{"line_number":2503,"context_line":"            call(RBD_FEATURE_JOURNALING, True)])"},{"line_number":2504,"context_line":"        self.assertEqual(4, image.update_features.call_count)"},{"line_number":2505,"context_line":"        self.assertEqual({\u0027provider_location\u0027: None}, ret)"},{"line_number":2506,"context_line":""},{"line_number":2507,"context_line":""},{"line_number":2508,"context_line":"class ManagedRBDTestCase(test_driver.BaseDriverTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_aecab115","line":2505,"in_reply_to":"3fa7e38b_71b90214","updated":"2020-01-08 14:16:38.000000000","message":"Done","commit_id":"3e057db79bd95ba53671f92c74df79c4a08e791c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e9885ade383511bec44382defd12591b758373f0","unresolved":false,"context_lines":[{"line_number":247,"context_line":"            **{\u0027name\u0027: u\u0027volume-0000000d\u0027,"},{"line_number":248,"context_line":"               \u0027id\u0027: \u0027a99b24d9-7b44-4b44-ae21-c83fc8e142f6\u0027,"},{"line_number":249,"context_line":"               \u0027size\u0027: 7,"},{"line_number":250,"context_line":"               \u0027provider_location\u0027: \u0027{\"saved_features\": 125}\u0027})"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        self.snapshot \u003d fake_snapshot.fake_snapshot_obj("},{"line_number":253,"context_line":"            self.context, name\u003d\u0027snapshot-0000000a\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_4e4e9d90","line":250,"range":{"start_line":250,"start_character":56,"end_line":250,"end_character":59},"updated":"2020-01-08 14:44:13.000000000","message":"nit: Use RBD_FEATURE_XYZ instead","commit_id":"d8f6572d3d2ce5cc7a9d6cc17fefae9042b3d4cd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e9885ade383511bec44382defd12591b758373f0","unresolved":false,"context_lines":[{"line_number":2494,"context_line":"        # works as expected."},{"line_number":2495,"context_line":""},{"line_number":2496,"context_line":"        def fake_update_features(feature, value):"},{"line_number":2497,"context_line":"            if feature in (RBD_FEATURE_JOURNALING, RBD_FEATURE_OBJECT_MAP):"},{"line_number":2498,"context_line":"                raise self.mock_rbd.InvalidArgument()"},{"line_number":2499,"context_line":""},{"line_number":2500,"context_line":"        image \u003d self.mock_proxy.return_value.__enter__.return_value"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_11852459","line":2497,"range":{"start_line":2497,"start_character":27,"end_line":2497,"end_character":49},"updated":"2020-01-08 14:44:13.000000000","message":"-1: Journaling is not set on L2501-L2505, so this case will never happen, right?","commit_id":"d8f6572d3d2ce5cc7a9d6cc17fefae9042b3d4cd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":2494,"context_line":""},{"line_number":2495,"context_line":"        ret \u003d self.driver._enable_multiattach(self.volume_d)"},{"line_number":2496,"context_line":""},{"line_number":2497,"context_line":"        image.update_features.assert_has_calls(["},{"line_number":2498,"context_line":"            call(RBD_FEATURE_JOURNALING |"},{"line_number":2499,"context_line":"                 RBD_FEATURE_FAST_DIFF |"},{"line_number":2500,"context_line":"                 RBD_FEATURE_OBJECT_MAP |"},{"line_number":2501,"context_line":"                 RBD_FEATURE_EXCLUSIVE_LOCK, False),"},{"line_number":2502,"context_line":"        ])"},{"line_number":2503,"context_line":"        self.assertEqual(1, image.update_features.call_count)"},{"line_number":2504,"context_line":"        self.assertEqual("},{"line_number":2505,"context_line":"            {\u0027provider_location\u0027:"},{"line_number":2506,"context_line":"             \"{\\\"saved_features\\\":%s}\" % image_features}, ret)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_d8dd4fde","line":2503,"range":{"start_line":2497,"start_character":0,"end_line":2503,"end_character":61},"updated":"2020-01-10 09:23:17.000000000","message":"-1: When we know it\u0027s only one call we should use assert_called_once_with:\n\n        image.update_features.assert_called_once_with(\n            RBD_FEATURE_JOURNALING |\n            RBD_FEATURE_FAST_DIFF |\n            RBD_FEATURE_OBJECT_MAP |\n            RBD_FEATURE_EXCLUSIVE_LOCK,\n            False)","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":2511,"context_line":"        # works as expected."},{"line_number":2512,"context_line":""},{"line_number":2513,"context_line":"        def fake_update_features(feature, value):"},{"line_number":2514,"context_line":"            if feature in (RBD_FEATURE_JOURNALING, RBD_FEATURE_OBJECT_MAP):"},{"line_number":2515,"context_line":"                raise self.mock_rbd.InvalidArgument()"},{"line_number":2516,"context_line":""},{"line_number":2517,"context_line":"        image \u003d self.mock_proxy.return_value.__enter__.return_value"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_b8f09367","line":2514,"updated":"2020-01-10 09:23:17.000000000","message":"-1: This is not correct, in most cases we are no longer passing a single feature:\n\n  if feature \u0026 (RBD_FEATURE_JOURNALING | RBD_FEATURE_OBJECT_MAP)","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":2524,"context_line":"        image.features.return_value \u003d image_features"},{"line_number":2525,"context_line":""},{"line_number":2526,"context_line":"        with mock.patch.object(image, \u0027update_features\u0027,"},{"line_number":2527,"context_line":"                               new\u003dfake_update_features):"},{"line_number":2528,"context_line":"            ret \u003d self.driver._enable_multiattach(self.volume_d)"},{"line_number":2529,"context_line":""},{"line_number":2530,"context_line":"            self.assertEqual("}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_989277fb","line":2527,"updated":"2020-01-10 09:23:17.000000000","message":"Wouldn\u0027t it be better to check what the call to the update method was instead of changing it?  After all the raise shouldn\u0027t happen anymore.","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":2533,"context_line":""},{"line_number":2534,"context_line":"    @common_mocks"},{"line_number":2535,"context_line":"    def test_disable_multiattach(self):"},{"line_number":2536,"context_line":"        image \u003d self.mock_proxy.return_value.__enter__.return_value"},{"line_number":2537,"context_line":""},{"line_number":2538,"context_line":"        ret \u003d self.driver._disable_multiattach(self.volume_d)"},{"line_number":2539,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_98bb5773","line":2536,"updated":"2020-01-10 09:23:17.000000000","message":"?: Why aren\u0027t we setting the image features?","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":2537,"context_line":""},{"line_number":2538,"context_line":"        ret \u003d self.driver._disable_multiattach(self.volume_d)"},{"line_number":2539,"context_line":""},{"line_number":2540,"context_line":"        image.update_features.assert_has_calls([call(0, True)])"},{"line_number":2541,"context_line":"        self.assertEqual(1, image.update_features.call_count)"},{"line_number":2542,"context_line":"        self.assertEqual({\u0027provider_location\u0027: None}, ret)"},{"line_number":2543,"context_line":""},{"line_number":2544,"context_line":"    @common_mocks"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_188687bb","line":2541,"range":{"start_line":2540,"start_character":0,"end_line":2541,"end_character":61},"updated":"2020-01-10 09:23:17.000000000","message":"-1:\n\n        image.update_features.assert_called_once_with(0, True)","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":2542,"context_line":"        self.assertEqual({\u0027provider_location\u0027: None}, ret)"},{"line_number":2543,"context_line":""},{"line_number":2544,"context_line":"    @common_mocks"},{"line_number":2545,"context_line":"    def test_disable_multiattach_features_already_set(self):"},{"line_number":2546,"context_line":"        # Ensures that InvalidArgument handling for already set features"},{"line_number":2547,"context_line":"        # works as expected."},{"line_number":2548,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_b8be5361","line":2545,"updated":"2020-01-10 09:23:17.000000000","message":"-1: This isn\u0027t any different than the previous test.","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":2547,"context_line":"        # works as expected."},{"line_number":2548,"context_line":""},{"line_number":2549,"context_line":"        def fake_update_features(feature, value):"},{"line_number":2550,"context_line":"            if feature in (RBD_FEATURE_JOURNALING, RBD_FEATURE_OBJECT_MAP):"},{"line_number":2551,"context_line":"                raise self.mock_rbd.InvalidArgument()"},{"line_number":2552,"context_line":""},{"line_number":2553,"context_line":"        image \u003d self.mock_proxy.return_value.__enter__.return_value"},{"line_number":2554,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_38aa6319","line":2551,"range":{"start_line":2550,"start_character":0,"end_line":2551,"end_character":53},"updated":"2020-01-10 09:23:17.000000000","message":"-1: This would only work if we only set 1 feature, not if we set multiple.","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":2553,"context_line":"        image \u003d self.mock_proxy.return_value.__enter__.return_value"},{"line_number":2554,"context_line":""},{"line_number":2555,"context_line":"        with mock.patch.object(image, \u0027update_features\u0027,"},{"line_number":2556,"context_line":"                               new\u003dfake_update_features):"},{"line_number":2557,"context_line":"            ret \u003d self.driver._disable_multiattach(self.volume_d)"},{"line_number":2558,"context_line":""},{"line_number":2559,"context_line":"            self.assertEqual({\u0027provider_location\u0027: None}, ret)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_9860370f","line":2556,"updated":"2020-01-10 09:23:17.000000000","message":"?: Why don\u0027t we just check the values that are passed to the update?","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"}],"cinder/volume/drivers/rbd.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a09e5b538e4e0d6e84139a5e6c03f9b9f61b3a59","unresolved":false,"context_lines":[{"line_number":771,"context_line":"    def _enable_multiattach(self, volume):"},{"line_number":772,"context_line":"        # Order matters here, iteration takes feature dependencies into"},{"line_number":773,"context_line":"        # account."},{"line_number":774,"context_line":"        multipath_feature_exclusions \u003d {"},{"line_number":775,"context_line":"            \"journaling\": self.rbd.RBD_FEATURE_JOURNALING,"},{"line_number":776,"context_line":"            \"fast diff\": self.rbd.RBD_FEATURE_FAST_DIFF,"},{"line_number":777,"context_line":"            \"object map\": self.rbd.RBD_FEATURE_OBJECT_MAP,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_0bc5530a","line":774,"range":{"start_line":774,"start_character":39,"end_line":774,"end_character":40},"updated":"2019-11-14 18:47:00.000000000","message":"This has to be made a collections.OrderedDict if order matters here.","commit_id":"a1958dd3d2e07633a9b5000df35bba41ede20489"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e9885ade383511bec44382defd12591b758373f0","unresolved":false,"context_lines":[{"line_number":810,"context_line":"            for name, value in multipath_feature_exclusions.items():"},{"line_number":811,"context_line":"                if image_features \u0026 value:"},{"line_number":812,"context_line":"                    try:"},{"line_number":813,"context_line":"                        image.update_features(value, True)"},{"line_number":814,"context_line":"                    except self.rbd.InvalidArgument:"},{"line_number":815,"context_line":"                        LOG.info(\"disable multiattach: skipping already \""},{"line_number":816,"context_line":"                                 \"set multiattach feature: %s\","}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_91bef4fd","line":813,"updated":"2020-01-08 14:44:13.000000000","message":"-1: If I understand this correctly we could have skipped a value from setting it to False on L786 (because it was already False) yet we saved it as part of the `saved_features`, which means that we would be setting it to True here and therefore restoring it wrong when doing the disabling.","commit_id":"d8f6572d3d2ce5cc7a9d6cc17fefae9042b3d4cd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"65af22887a1b922a9683fdf7a919499e3b15a4ed","unresolved":false,"context_lines":[{"line_number":810,"context_line":"            for name, value in multipath_feature_exclusions.items():"},{"line_number":811,"context_line":"                if image_features \u0026 value:"},{"line_number":812,"context_line":"                    try:"},{"line_number":813,"context_line":"                        image.update_features(value, True)"},{"line_number":814,"context_line":"                    except self.rbd.InvalidArgument:"},{"line_number":815,"context_line":"                        LOG.info(\"disable multiattach: skipping already \""},{"line_number":816,"context_line":"                                 \"set multiattach feature: %s\","}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_0344ec46","line":813,"in_reply_to":"3fa7e38b_04187981","updated":"2020-01-09 09:40:52.000000000","message":"Oh, thanks for the clarification!!\n\nThen if that\u0027s the reason for the try...except clause, why don\u0027t we just drop the whole for loop and make a single call to update_features?\n\nOn driver\u0027s __init__ we do:\n        self.MULTIATTACH_FEATURES \u003d (self.rbd.RBD_FEATURE_JOURNALING | \n                                     self.rbd.RBD_FEATURE_FAST_DIFF |\n                                     self.rbd.RBD_FEATURE_OBJECT_MAP | \n                                     self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK)\n\n\nAnd the whole _enable_multiattach function can be:\n        vol_name \u003d utils.convert_str(volume.name)\n        with RBDVolumeProxy(self, vol_name) as image:\n            image_features \u003d image.features()\n            change_features \u003d self.MULTIATTACH_FEATURES \u0026 image_features\n            image.update_features(change_features, False)\n\n        return {\u0027provider_location\u0027:\n                self._dumps({\u0027saved_features\u0027: image_features})}\n\n\nAnd to disable we also wouldn\u0027t need a loop (L810-L817):\n        change_features \u003d  self.MULTIATTACH_FEATURES \u0026 image_features\n        image.update_features(change_features, True)\n\n\nOr am I missing something?","commit_id":"d8f6572d3d2ce5cc7a9d6cc17fefae9042b3d4cd"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"b0a58710acc5f3bb644cc05f396d0defbb78ed2c","unresolved":false,"context_lines":[{"line_number":810,"context_line":"            for name, value in multipath_feature_exclusions.items():"},{"line_number":811,"context_line":"                if image_features \u0026 value:"},{"line_number":812,"context_line":"                    try:"},{"line_number":813,"context_line":"                        image.update_features(value, True)"},{"line_number":814,"context_line":"                    except self.rbd.InvalidArgument:"},{"line_number":815,"context_line":"                        LOG.info(\"disable multiattach: skipping already \""},{"line_number":816,"context_line":"                                 \"set multiattach feature: %s\","}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_04187981","line":813,"in_reply_to":"3fa7e38b_91bef4fd","updated":"2020-01-09 02:25:47.000000000","message":"No, see line 806 combined with line 811.  We have the original bitmask; for each targeted feature in that bitmask, if it was enabled, we attempt to reenable it.  The unobvious bit is that update_features() *may* change other features in order to satisfy the request, so the try/catch addresses features that needed setting but were already set by update_features().  This goes for _enable_multiattach() as well.","commit_id":"d8f6572d3d2ce5cc7a9d6cc17fefae9042b3d4cd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":770,"context_line":"                \u0027replication_driver_data\u0027: driver_data}"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"    def _enable_multiattach(self, volume):"},{"line_number":773,"context_line":"        MULTIATTACH_FEATURES \u003d (self.rbd.RBD_FEATURE_JOURNALING |"},{"line_number":774,"context_line":"                                self.rbd.RBD_FEATURE_FAST_DIFF |"},{"line_number":775,"context_line":"                                self.rbd.RBD_FEATURE_OBJECT_MAP |"},{"line_number":776,"context_line":"                                self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_186a071a","line":773,"updated":"2020-01-10 09:23:17.000000000","message":"nit: Creating this as an instance attribute on __init__ method means that we don\u0027t have to create the tuple each time.","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":784,"context_line":"                self._dumps({\u0027saved_features\u0027: image_features})}"},{"line_number":785,"context_line":""},{"line_number":786,"context_line":"    def _disable_multiattach(self, volume):"},{"line_number":787,"context_line":"        MULTIATTACH_FEATURES \u003d (self.rbd.RBD_FEATURE_JOURNALING |"},{"line_number":788,"context_line":"                                self.rbd.RBD_FEATURE_FAST_DIFF |"},{"line_number":789,"context_line":"                                self.rbd.RBD_FEATURE_OBJECT_MAP |"},{"line_number":790,"context_line":"                                self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK)"},{"line_number":791,"context_line":"        vol_name \u003d utils.convert_str(volume.name)"},{"line_number":792,"context_line":"        with RBDVolumeProxy(self, vol_name) as image:"},{"line_number":793,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_38658349","line":790,"range":{"start_line":787,"start_character":0,"end_line":790,"end_character":68},"updated":"2020-01-10 09:23:17.000000000","message":"-1: I don\u0027t see a reason to have this duplicated here, not only is it less efficient than having it in the __init__ method, but it also adds more code.\n\nWhy are we duplicating it? What am I missing?","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":799,"context_line":"                msg \u003d \"Failed to restore image features.\""},{"line_number":800,"context_line":"                raise RBDDriverException(reason\u003dmsg)"},{"line_number":801,"context_line":"            except Exception:"},{"line_number":802,"context_line":"                msg \u003d \"Could not find saved image features.\""},{"line_number":803,"context_line":"                raise RBDDriverException(reason\u003dmsg)"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":"        return {\u0027provider_location\u0027: None}"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_5855df50","line":802,"updated":"2020-01-10 09:23:17.000000000","message":"?: Couldn\u0027t this be misleading if the error is a network connection issue on the update call?","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"40eb3d5fbc0629d65060a7e87885913ef946303b","unresolved":false,"context_lines":[{"line_number":799,"context_line":"                msg \u003d \"Failed to restore image features.\""},{"line_number":800,"context_line":"                raise RBDDriverException(reason\u003dmsg)"},{"line_number":801,"context_line":"            except Exception:"},{"line_number":802,"context_line":"                msg \u003d \"Could not find saved image features.\""},{"line_number":803,"context_line":"                raise RBDDriverException(reason\u003dmsg)"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":"        return {\u0027provider_location\u0027: None}"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_d9a535a2","line":802,"in_reply_to":"3fa7e38b_5855df50","updated":"2020-01-13 20:22:02.000000000","message":"See the above exception for rbd.InvalidArgument, that should take care of it.","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"    def _is_replicated_type(self, volume_type):"},{"line_number":808,"context_line":"        try:"},{"line_number":809,"context_line":"            extra_specs \u003d volume_type[\u0027extra_specs\u0027]"},{"line_number":810,"context_line":"            LOG.debug(\u0027extra_specs: %s\u0027, extra_specs)"},{"line_number":811,"context_line":"            return extra_specs.get(EXTRA_SPECS_REPL_ENABLED) \u003d\u003d \"\u003cis\u003e True\""},{"line_number":812,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_98c4b70f","line":809,"updated":"2020-01-10 09:23:17.000000000","message":"Better use attribute notation so we know that volume_type is not a dict:\n\n  volume_type.extra_specs","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"40eb3d5fbc0629d65060a7e87885913ef946303b","unresolved":false,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"    def _is_replicated_type(self, volume_type):"},{"line_number":808,"context_line":"        try:"},{"line_number":809,"context_line":"            extra_specs \u003d volume_type[\u0027extra_specs\u0027]"},{"line_number":810,"context_line":"            LOG.debug(\u0027extra_specs: %s\u0027, extra_specs)"},{"line_number":811,"context_line":"            return extra_specs.get(EXTRA_SPECS_REPL_ENABLED) \u003d\u003d \"\u003cis\u003e True\""},{"line_number":812,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_b9b639db","line":809,"in_reply_to":"3fa7e38b_98c4b70f","updated":"2020-01-13 20:22:02.000000000","message":"Have you tried this in python3?","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b88772e9345607cfd25641c3c9c0e075e0d9369","unresolved":false,"context_lines":[{"line_number":815,"context_line":""},{"line_number":816,"context_line":"    def _is_multiattach_type(self, volume_type):"},{"line_number":817,"context_line":"        try:"},{"line_number":818,"context_line":"            extra_specs \u003d volume_type[\u0027extra_specs\u0027]"},{"line_number":819,"context_line":"            LOG.debug(\u0027extra_specs: %s\u0027, extra_specs)"},{"line_number":820,"context_line":"            return extra_specs.get(EXTRA_SPECS_MULTIATTACH) \u003d\u003d \"\u003cis\u003e True\""},{"line_number":821,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_58cabf18","line":818,"updated":"2020-01-10 09:23:17.000000000","message":"ditto","commit_id":"6df7a3da89b9984c4a6e7552c932708c5808aee2"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"eda474130933716acff83f55a79920b2d650fa3d","unresolved":false,"context_lines":[{"line_number":805,"context_line":"            try:"},{"line_number":806,"context_line":"                provider_location \u003d json.loads(volume.provider_location)"},{"line_number":807,"context_line":"                image_features \u003d provider_location[\u0027saved_features\u0027]"},{"line_number":808,"context_line":"                change_features \u003d self.MULTIATTACH_EXCLUSIONS \u0026 image_features"},{"line_number":809,"context_line":"                image.update_features(change_features, True)"},{"line_number":810,"context_line":"            except IndexError:"},{"line_number":811,"context_line":"                msg \u003d \"Could not find saved image features.\""}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_d8e38439","line":808,"range":{"start_line":808,"start_character":62,"end_line":808,"end_character":63},"updated":"2020-01-20 22:05:05.000000000","message":"Is ANDing right with the multiattach exclusions? So it will only change things that are in the exclusion list? That seems like it conflicts with the naming of that.\n\nNot sure if we need a comment here explaining what is being done better, or if I am just not getting it.","commit_id":"9bc67c8978faba9e879bf377652d2679e143d992"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"24d7001995077a09300aaa4cafe2d35105c51bc5","unresolved":false,"context_lines":[{"line_number":805,"context_line":"            try:"},{"line_number":806,"context_line":"                provider_location \u003d json.loads(volume.provider_location)"},{"line_number":807,"context_line":"                image_features \u003d provider_location[\u0027saved_features\u0027]"},{"line_number":808,"context_line":"                change_features \u003d self.MULTIATTACH_EXCLUSIONS \u0026 image_features"},{"line_number":809,"context_line":"                image.update_features(change_features, True)"},{"line_number":810,"context_line":"            except IndexError:"},{"line_number":811,"context_line":"                msg \u003d \"Could not find saved image features.\""}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_4ead87e5","line":808,"range":{"start_line":808,"start_character":62,"end_line":808,"end_character":63},"in_reply_to":"3fa7e38b_d8e38439","updated":"2020-01-21 12:36:43.000000000","message":"Yes, this is correct logic.  We acquire a bitmask of only the features changed from the enable call.  Here we reenable those features as this is the \"disable_multiattach\" method, so we\u0027re removing the exclusions that were applied as a result of the enable method.","commit_id":"9bc67c8978faba9e879bf377652d2679e143d992"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"eda474130933716acff83f55a79920b2d650fa3d","unresolved":false,"context_lines":[{"line_number":810,"context_line":"            except IndexError:"},{"line_number":811,"context_line":"                msg \u003d \"Could not find saved image features.\""},{"line_number":812,"context_line":"                raise RBDDriverException(reason\u003dmsg)"},{"line_number":813,"context_line":"            except self.rbd.InvalidArgument:"},{"line_number":814,"context_line":"                msg \u003d \"Failed to restore image features.\""},{"line_number":815,"context_line":"                raise RBDDriverException(reason\u003dmsg)"},{"line_number":816,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_3887385e","line":813,"updated":"2020-01-20 22:05:05.000000000","message":"I\u0027m a little confused on this since from the commit message I thought it was saying InvalidArgument is actually OK since it will be raised if the flag is already set to what is being requested. So raising a driver exception looks like it will still cause a failure like it did prior to the change.","commit_id":"9bc67c8978faba9e879bf377652d2679e143d992"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"24d7001995077a09300aaa4cafe2d35105c51bc5","unresolved":false,"context_lines":[{"line_number":810,"context_line":"            except IndexError:"},{"line_number":811,"context_line":"                msg \u003d \"Could not find saved image features.\""},{"line_number":812,"context_line":"                raise RBDDriverException(reason\u003dmsg)"},{"line_number":813,"context_line":"            except self.rbd.InvalidArgument:"},{"line_number":814,"context_line":"                msg \u003d \"Failed to restore image features.\""},{"line_number":815,"context_line":"                raise RBDDriverException(reason\u003dmsg)"},{"line_number":816,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_8ea7ff09","line":813,"in_reply_to":"3fa7e38b_3887385e","updated":"2020-01-21 12:36:43.000000000","message":"InvalidArgument would be valid for a single feature that had already been disabled as a dependency of another feature as a side effect.  But in this case we\u0027re making a single call and there should never be an InvalidArgument exception in that case.  If this happens, the operation has failed and there is no recovery.","commit_id":"9bc67c8978faba9e879bf377652d2679e143d992"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"eda474130933716acff83f55a79920b2d650fa3d","unresolved":false,"context_lines":[{"line_number":817,"context_line":"        return {\u0027provider_location\u0027: None}"},{"line_number":818,"context_line":""},{"line_number":819,"context_line":"    def _is_replicated_type(self, volume_type):"},{"line_number":820,"context_line":"        try:"},{"line_number":821,"context_line":"            extra_specs \u003d volume_type.extra_specs"},{"line_number":822,"context_line":"            LOG.debug(\u0027extra_specs: %s\u0027, extra_specs)"},{"line_number":823,"context_line":"            return extra_specs.get(EXTRA_SPECS_REPL_ENABLED) \u003d\u003d \"\u003cis\u003e True\""}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_b8a248fa","line":820,"updated":"2020-01-20 22:05:05.000000000","message":"Are these two _is_*_type changes related? It doesn\u0027t look like this actually changes anything, other than adding a debug log. But not sure I see how an exception can get thrown since the get call on extra_specs should just return None, and \u0027None \u003d\u003d \"\u003cis\u003e True\"\u0027 should just return the expected False.","commit_id":"9bc67c8978faba9e879bf377652d2679e143d992"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"24d7001995077a09300aaa4cafe2d35105c51bc5","unresolved":false,"context_lines":[{"line_number":817,"context_line":"        return {\u0027provider_location\u0027: None}"},{"line_number":818,"context_line":""},{"line_number":819,"context_line":"    def _is_replicated_type(self, volume_type):"},{"line_number":820,"context_line":"        try:"},{"line_number":821,"context_line":"            extra_specs \u003d volume_type.extra_specs"},{"line_number":822,"context_line":"            LOG.debug(\u0027extra_specs: %s\u0027, extra_specs)"},{"line_number":823,"context_line":"            return extra_specs.get(EXTRA_SPECS_REPL_ENABLED) \u003d\u003d \"\u003cis\u003e True\""}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_6eaac3d3","line":820,"in_reply_to":"3fa7e38b_b8a248fa","updated":"2020-01-21 12:36:43.000000000","message":"This failed to work in a recent devstack environment and dot notation should be fine for this OVO.  As extra specs are not guaranteed to be present, an AttributeError exeption is possible, which should be handled.","commit_id":"9bc67c8978faba9e879bf377652d2679e143d992"}],"releasenotes/notes/rbd-multiattach-exceptions-43066312f3b527f5.yaml":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6a38d12498fd80a774cc0e406b61ce9286bd1196","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":""},{"line_number":5,"context_line":"    Catch argument exceptions when configuring multiattach for rbd volumes."},{"line_number":6,"context_line":"    This allows multiattach images with flags already set to continue instead"},{"line_number":7,"context_line":"    of raising an exception and failing."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"3fa7e38b_912a7e75","line":4,"updated":"2020-01-07 22:06:59.000000000","message":"nit: don\u0027t need this blank line","commit_id":"3e057db79bd95ba53671f92c74df79c4a08e791c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"5a4fdb57ab123ab95dac5acab46c2563be4969b8","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":""},{"line_number":5,"context_line":"    Catch argument exceptions when configuring multiattach for rbd volumes."},{"line_number":6,"context_line":"    This allows multiattach images with flags already set to continue instead"},{"line_number":7,"context_line":"    of raising an exception and failing."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"3fa7e38b_4ed9bdee","line":4,"in_reply_to":"3fa7e38b_912a7e75","updated":"2020-01-08 14:16:38.000000000","message":"Done","commit_id":"3e057db79bd95ba53671f92c74df79c4a08e791c"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"eda474130933716acff83f55a79920b2d650fa3d","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Catch argument exceptions when configuring multiattach for rbd volumes."},{"line_number":5,"context_line":"    This allows multiattach images with flags already set to continue instead"},{"line_number":6,"context_line":"    of raising an exception and failing."}],"source_content_type":"text/x-yaml","patch_set":7,"id":"3fa7e38b_d8470452","line":6,"updated":"2020-01-20 22:05:05.000000000","message":"Might be nice to include a link to the bug report for more context.","commit_id":"9bc67c8978faba9e879bf377652d2679e143d992"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"24d7001995077a09300aaa4cafe2d35105c51bc5","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Catch argument exceptions when configuring multiattach for rbd volumes."},{"line_number":5,"context_line":"    This allows multiattach images with flags already set to continue instead"},{"line_number":6,"context_line":"    of raising an exception and failing."}],"source_content_type":"text/x-yaml","patch_set":7,"id":"3fa7e38b_6e93a3a5","line":6,"in_reply_to":"3fa7e38b_d8470452","updated":"2020-01-21 12:36:43.000000000","message":"I\u0027ll include that in the next update.","commit_id":"9bc67c8978faba9e879bf377652d2679e143d992"}]}