)]}'
{"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8f14e2e9963101fcd27c7f205763df5aad6805d4","unresolved":false,"context_lines":[{"line_number":6461,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor\u0027)"},{"line_number":6462,"context_line":"    def test_use_encryptor_connection_info_incomplete(self,"},{"line_number":6463,"context_line":"            mock_get_encryptor, mock_get_metadata):"},{"line_number":6464,"context_line":"        \"\"\"Assert no attach attempt is made given incomplete connection_info."},{"line_number":6465,"context_line":"        \"\"\""},{"line_number":6466,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":6467,"context_line":"        connection_info \u003d {\u0027data\u0027: {}}"},{"line_number":6468,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_70209af8","line":6465,"range":{"start_line":6464,"start_character":8,"end_line":6465,"end_character":11},"updated":"2017-05-03 11:33:06.000000000","message":"I wish these were everywhere","commit_id":"3bc44a30671691c4b09aef977650e41ecc767881"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8f14e2e9963101fcd27c7f205763df5aad6805d4","unresolved":false,"context_lines":[{"line_number":6475,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor\u0027)"},{"line_number":6476,"context_line":"    def test_attach_encryptor_unencrypted_volume_meta_missing(self,"},{"line_number":6477,"context_line":"            mock_get_encryptor, mock_get_metadata):"},{"line_number":6478,"context_line":"        \"\"\"Assert that if not provided encryption metadata is fetched even"},{"line_number":6479,"context_line":"           if the volume is ultimately unencrypted and no attempt to attach"},{"line_number":6480,"context_line":"           is made."},{"line_number":6481,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_3061f23a","line":6478,"range":{"start_line":6478,"start_character":23,"end_line":6478,"end_character":38},"updated":"2017-05-03 11:33:06.000000000","message":"grammar nit:\n\n   Assert that, if not provided, encryption metadata...\n\nIt took me two or three attempts to parse that :) Ditto for many of the docstrings below.","commit_id":"3bc44a30671691c4b09aef977650e41ecc767881"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7d273bead0fbfba0428d9c93875e03a1cd1b3aa5","unresolved":false,"context_lines":[{"line_number":6821,"context_line":""},{"line_number":6822,"context_line":"    @mock.patch(\u0027os_brick.encryptors.get_encryption_metadata\u0027)"},{"line_number":6823,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor\u0027)"},{"line_number":6824,"context_line":"    def test_use_encryptor_connection_info_incomplete(self,"},{"line_number":6825,"context_line":"            mock_get_encryptor, mock_get_metadata):"},{"line_number":6826,"context_line":"        \"\"\"Assert no attach attempt is made given incomplete connection_info."},{"line_number":6827,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_805d9048","line":6824,"range":{"start_line":6824,"start_character":13,"end_line":6824,"end_character":16},"updated":"2017-12-18 11:19:55.000000000","message":"s/use/attach/?","commit_id":"63ee24ebc5fda545e7e2ee5aa984325368ddd334"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7d273bead0fbfba0428d9c93875e03a1cd1b3aa5","unresolved":false,"context_lines":[{"line_number":6823,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor\u0027)"},{"line_number":6824,"context_line":"    def test_use_encryptor_connection_info_incomplete(self,"},{"line_number":6825,"context_line":"            mock_get_encryptor, mock_get_metadata):"},{"line_number":6826,"context_line":"        \"\"\"Assert no attach attempt is made given incomplete connection_info."},{"line_number":6827,"context_line":"        \"\"\""},{"line_number":6828,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":6829,"context_line":"        connection_info \u003d {\u0027data\u0027: {}}"},{"line_number":6830,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_40a54846","line":6827,"range":{"start_line":6826,"start_character":0,"end_line":6827,"end_character":11},"updated":"2017-12-18 11:19:55.000000000","message":"Nit: There\u0027s some reason we don\u0027t use docstrings here. Something to do with unwanted doc generation, perhaps? If this was really banned, though, I guess there should be a HACKING rule for it.","commit_id":"63ee24ebc5fda545e7e2ee5aa984325368ddd334"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"2e33462eed44346a02c3b65d02daa0f7bfb6c3e1","unresolved":false,"context_lines":[{"line_number":6823,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor\u0027)"},{"line_number":6824,"context_line":"    def test_use_encryptor_connection_info_incomplete(self,"},{"line_number":6825,"context_line":"            mock_get_encryptor, mock_get_metadata):"},{"line_number":6826,"context_line":"        \"\"\"Assert no attach attempt is made given incomplete connection_info."},{"line_number":6827,"context_line":"        \"\"\""},{"line_number":6828,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":6829,"context_line":"        connection_info \u003d {\u0027data\u0027: {}}"},{"line_number":6830,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_cd0bb627","line":6827,"range":{"start_line":6826,"start_character":0,"end_line":6827,"end_character":11},"in_reply_to":"df87a7cf_40a54846","updated":"2017-12-19 09:30:23.000000000","message":"Apparently this was something to do with nose, which we don\u0027t use any more. Docstrings are back in, I think.","commit_id":"63ee24ebc5fda545e7e2ee5aa984325368ddd334"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7d273bead0fbfba0428d9c93875e03a1cd1b3aa5","unresolved":false,"context_lines":[{"line_number":6819,"context_line":"                          drvr.extend_volume,"},{"line_number":6820,"context_line":"                          connection_info, instance)"},{"line_number":6821,"context_line":""},{"line_number":6822,"context_line":"    @mock.patch(\u0027os_brick.encryptors.get_encryption_metadata\u0027)"},{"line_number":6823,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor\u0027)"},{"line_number":6824,"context_line":"    def test_use_encryptor_connection_info_incomplete(self,"},{"line_number":6825,"context_line":"            mock_get_encryptor, mock_get_metadata):"},{"line_number":6826,"context_line":"        \"\"\"Assert no attach attempt is made given incomplete connection_info."},{"line_number":6827,"context_line":"        \"\"\""},{"line_number":6828,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":6829,"context_line":"        connection_info \u003d {\u0027data\u0027: {}}"},{"line_number":6830,"context_line":""},{"line_number":6831,"context_line":"        drvr._attach_encryptor(self.context, connection_info)"}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_207da4b1","line":6828,"range":{"start_line":6822,"start_character":0,"end_line":6828,"end_character":70},"updated":"2017-12-18 11:19:55.000000000","message":"Suggestion: This code is common amongst all these tests, and you don\u0027t seem to be using any of the other initialisation of this test class. Suggests these tests might live more comfortably in their own test class? Our tests classes are generally way too big.","commit_id":"63ee24ebc5fda545e7e2ee5aa984325368ddd334"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7d273bead0fbfba0428d9c93875e03a1cd1b3aa5","unresolved":false,"context_lines":[{"line_number":6844,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":6845,"context_line":"        encryption \u003d {}"},{"line_number":6846,"context_line":"        connection_info \u003d {\u0027data\u0027: {\u0027volume_id\u0027: uuids.volume_id}}"},{"line_number":6847,"context_line":"        mock_get_metadata.return_value \u003d encryption"},{"line_number":6848,"context_line":""},{"line_number":6849,"context_line":"        drvr._attach_encryptor(self.context, connection_info)"},{"line_number":6850,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_8b105150","line":6847,"range":{"start_line":6847,"start_character":0,"end_line":6847,"end_character":51},"updated":"2017-12-18 11:19:55.000000000","message":"Eww... it really does return {} instead of None.","commit_id":"63ee24ebc5fda545e7e2ee5aa984325368ddd334"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"355d1fa734491c6c838f935b875a4b1cd683a098","unresolved":false,"context_lines":[{"line_number":6869,"context_line":"    def test_attach_encryptor_unencrypted_volume_meta_missing(self,"},{"line_number":6870,"context_line":"            mock_get_encryptor, mock_get_metadata):"},{"line_number":6871,"context_line":"        \"\"\"Assert that if not provided encryption metadata is fetched even"},{"line_number":6872,"context_line":"           if the volume is ultimately unencrypted and no attempt to attach"},{"line_number":6873,"context_line":"           is made."},{"line_number":6874,"context_line":"        \"\"\""},{"line_number":6875,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":6876,"context_line":"        encryption \u003d {}"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_d9c90926","line":6873,"range":{"start_line":6872,"start_character":0,"end_line":6873,"end_character":19},"updated":"2018-01-10 11:51:23.000000000","message":"nit: indentation.\n\n(Although actually I think I prefer this, but it\u0027s inconsistent.)","commit_id":"169a238530eb46eb3eb3b214abf4b90a64584239"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"355d1fa734491c6c838f935b875a4b1cd683a098","unresolved":false,"context_lines":[{"line_number":16307,"context_line":"    def test_migrate_disk_and_power_off(self):"},{"line_number":16308,"context_line":"        flavor \u003d {\u0027root_gb\u0027: 10, \u0027ephemeral_gb\u0027: 20}"},{"line_number":16309,"context_line":"        flavor_obj \u003d objects.Flavor(**flavor)"},{"line_number":16310,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":16311,"context_line":"        self._test_migrate_disk_and_power_off(ctxt, flavor_obj)"},{"line_number":16312,"context_line":""},{"line_number":16313,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_597979c5","line":16310,"range":{"start_line":16310,"start_character":0,"end_line":16310,"end_character":42},"updated":"2018-01-10 11:51:23.000000000","message":"self.context","commit_id":"169a238530eb46eb3eb3b214abf4b90a64584239"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"355d1fa734491c6c838f935b875a4b1cd683a098","unresolved":false,"context_lines":[{"line_number":16324,"context_line":"        flavor \u003d {\u0027root_gb\u0027: 1, \u0027ephemeral_gb\u0027: 0}"},{"line_number":16325,"context_line":"        flavor_obj \u003d objects.Flavor(**flavor)"},{"line_number":16326,"context_line":"        # Note(Mike_D): The size of instance\u0027s ephemeral_gb is 0 gb."},{"line_number":16327,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":16328,"context_line":"        self._test_migrate_disk_and_power_off(ctxt,"},{"line_number":16329,"context_line":"            flavor_obj, block_device_info\u003dinfo,"},{"line_number":16330,"context_line":"            params_for_instance\u003d{\u0027image_ref\u0027: None,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_397a75b9","line":16327,"updated":"2018-01-10 11:51:23.000000000","message":"self.context","commit_id":"169a238530eb46eb3eb3b214abf4b90a64584239"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"355d1fa734491c6c838f935b875a4b1cd683a098","unresolved":false,"context_lines":[{"line_number":16350,"context_line":"                 \u0027connection_info\u0027: mock.sentinel.conn_info_vda}]}"},{"line_number":16351,"context_line":"        flavor \u003d {\u0027root_gb\u0027: 1, \u0027ephemeral_gb\u0027: 0}"},{"line_number":16352,"context_line":"        flavor_obj \u003d objects.Flavor(**flavor)"},{"line_number":16353,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":16354,"context_line":"        self._test_migrate_disk_and_power_off(ctxt,"},{"line_number":16355,"context_line":"            flavor_obj, block_device_info\u003dinfo,"},{"line_number":16356,"context_line":"            params_for_instance\u003d{"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_d94ee995","line":16353,"updated":"2018-01-10 11:51:23.000000000","message":"self.context","commit_id":"169a238530eb46eb3eb3b214abf4b90a64584239"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"355d1fa734491c6c838f935b875a4b1cd683a098","unresolved":false,"context_lines":[{"line_number":16626,"context_line":"        # Old flavor, eph is 20, real disk is 3, target is 4"},{"line_number":16627,"context_line":"        flavor \u003d {\u0027root_gb\u0027: 10, \u0027ephemeral_gb\u0027: 4}"},{"line_number":16628,"context_line":"        flavor_obj \u003d objects.Flavor(**flavor)"},{"line_number":16629,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":16630,"context_line":"        self._test_migrate_disk_and_power_off(ctxt, flavor_obj)"},{"line_number":16631,"context_line":""},{"line_number":16632,"context_line":"    @mock.patch(\u0027nova.utils.execute\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_f9516d37","line":16629,"updated":"2018-01-10 11:51:23.000000000","message":"self.context","commit_id":"169a238530eb46eb3eb3b214abf4b90a64584239"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"88b0e75a2579016533593273122da64055822796","unresolved":false,"context_lines":[{"line_number":1171,"context_line":"        to determine if an attempt to attach the encryptor should be made."},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"        \"\"\""},{"line_number":1174,"context_line":"        if encryption is None and (\u0027data\u0027 in connection_info and"},{"line_number":1175,"context_line":"                \u0027volume_id\u0027 in connection_info[\u0027data\u0027]):"},{"line_number":1176,"context_line":"            volume_id \u003d connection_info[\u0027data\u0027][\u0027volume_id\u0027]"},{"line_number":1177,"context_line":"            encryption \u003d encryptors.get_encryption_metadata(context,"},{"line_number":1178,"context_line":"                            self._volume_api, volume_id, connection_info)"},{"line_number":1179,"context_line":""},{"line_number":1180,"context_line":"        if encryption:"},{"line_number":1181,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":1182,"context_line":"                                                   encryption)"},{"line_number":1183,"context_line":"            encryptor.attach_volume(context, **encryption)"},{"line_number":1184,"context_line":""},{"line_number":1185,"context_line":"    def _detach_encryptor(self, context, connection_info, encryption\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f013ff3_3c3ac833","line":1182,"range":{"start_line":1174,"start_character":0,"end_line":1182,"end_character":62},"updated":"2017-05-16 16:35:06.000000000","message":"nit: This is identical between these 2 functions. You could abstract this even further.","commit_id":"557408cc3971691613e0b1065a93ed0e430bfd0e"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"88b0e75a2579016533593273122da64055822796","unresolved":false,"context_lines":[{"line_number":1386,"context_line":"            # as we already have the encryption metadata dict from the compute"},{"line_number":1387,"context_line":"            # layer. This avoids the need to add the request context to the"},{"line_number":1388,"context_line":"            # signature of detach_volume requiring changes across all drivers."},{"line_number":1389,"context_line":"            self._detach_encryptor(None, connection_info, encryption)"},{"line_number":1390,"context_line":""},{"line_number":1391,"context_line":"        except exception.InstanceNotFound:"},{"line_number":1392,"context_line":"            # NOTE(zhaoqin): If the instance does not exist, _lookup_by_name()"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f013ff3_48e71314","line":1389,"updated":"2017-05-16 16:35:06.000000000","message":"Note: if we weren\u0027t passed encryption, this now results in an additional trip into encryptors.get_encryption_metadata(). However, this short circuits if \u0027encrypted\u0027 isn\u0027t in the connection_info, so this is fine.","commit_id":"557408cc3971691613e0b1065a93ed0e430bfd0e"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7d273bead0fbfba0428d9c93875e03a1cd1b3aa5","unresolved":false,"context_lines":[{"line_number":1191,"context_line":"                                               connection_info\u003dconnection_info,"},{"line_number":1192,"context_line":"                                               **encryption)"},{"line_number":1193,"context_line":""},{"line_number":1194,"context_line":"    def _get_volume_encryption(self, context, connection_info, encryption):"},{"line_number":1195,"context_line":"        \"\"\"Get the encryption metadata dict if it is not provided"},{"line_number":1196,"context_line":"        \"\"\""},{"line_number":1197,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_c05b98f3","line":1194,"updated":"2017-12-18 11:19:55.000000000","message":"Trivial suggestion: if you folded this into _get_volume_encryptor and made that method return None when there isn\u0027t one, we\u0027d only have a single function called _get_volume_encrypt*, and _(attach|detach)_encryptor would have a couple of extra lines of common code.","commit_id":"63ee24ebc5fda545e7e2ee5aa984325368ddd334"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7d273bead0fbfba0428d9c93875e03a1cd1b3aa5","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"        \"\"\"Get the encryption metadata dict if it is not provided"},{"line_number":1196,"context_line":"        \"\"\""},{"line_number":1197,"context_line":""},{"line_number":1198,"context_line":"        if encryption is None and (\u0027data\u0027 in connection_info and"},{"line_number":1199,"context_line":"                \u0027volume_id\u0027 in connection_info[\u0027data\u0027]):"},{"line_number":1200,"context_line":"            volume_id \u003d connection_info[\u0027data\u0027][\u0027volume_id\u0027]"},{"line_number":1201,"context_line":"            encryption \u003d encryptors.get_encryption_metadata(context,"},{"line_number":1202,"context_line":"                            self._volume_api, volume_id, connection_info)"},{"line_number":1203,"context_line":"        return encryption"}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_e0d33c37","line":1200,"range":{"start_line":1198,"start_character":34,"end_line":1200,"end_character":60},"updated":"2017-12-18 11:19:55.000000000","message":"Suggestion only: I\u0027m not -1 on this because this is simply code motion.\n\nThis is an instance where Nova is looking inside connection_info, which is generally undesirable. In the context of https://review.openstack.org/#/c/528363/ I wonder if it wouldn\u0027t be better to pass in the BDM (well, the DriverBlockDevice) here. We could check it directly to see if it\u0027s a volume and pull the volume_id from the BDM. No looking inside supposedly opaque foreign dicts required.","commit_id":"63ee24ebc5fda545e7e2ee5aa984325368ddd334"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"736a984132dfc5887fb90f3acc0004e2acc74f5f","unresolved":false,"context_lines":[{"line_number":1209,"context_line":"        to determine if an attempt to attach the encryptor should be made."},{"line_number":1210,"context_line":"        \"\"\""},{"line_number":1211,"context_line":"        encryption \u003d self._get_volume_encryption(context, connection_info,"},{"line_number":1212,"context_line":"                                                 encryption)"},{"line_number":1213,"context_line":"        if encryption:"},{"line_number":1214,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":1215,"context_line":"                                                   encryption)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_12194a78","line":1212,"range":{"start_line":1212,"start_character":49,"end_line":1212,"end_character":59},"updated":"2017-12-21 00:09:15.000000000","message":"This seems strange/unexpected. If you pass encryption meta, it will no-op and just return it to you, else it will query for encryption meta. I think the api would be more intuitive and user-friendly if _get_volume_encryption always queried for encryption meta and you don\u0027t pass encryption meta to it, i.e.:\n\n if encryption is None:\n     encryption \u003d self._get_volume_encryption(context, connection_info)","commit_id":"7d7c0131a5c3a9819a8cfc875bded71056eabb13"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"315d464c0295dfd2df06d01ebd12d7511090c6fa","unresolved":false,"context_lines":[{"line_number":1209,"context_line":"        to determine if an attempt to attach the encryptor should be made."},{"line_number":1210,"context_line":"        \"\"\""},{"line_number":1211,"context_line":"        encryption \u003d self._get_volume_encryption(context, connection_info,"},{"line_number":1212,"context_line":"                                                 encryption)"},{"line_number":1213,"context_line":"        if encryption:"},{"line_number":1214,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":1215,"context_line":"                                                   encryption)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_b27173e3","line":1212,"range":{"start_line":1212,"start_character":49,"end_line":1212,"end_character":59},"in_reply_to":"bf8cb3f7_12194a78","updated":"2018-01-08 21:12:55.000000000","message":"Thanks,","commit_id":"7d7c0131a5c3a9819a8cfc875bded71056eabb13"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"736a984132dfc5887fb90f3acc0004e2acc74f5f","unresolved":false,"context_lines":[{"line_number":1223,"context_line":"        to determine if an attempt to detach the encryptor should be made."},{"line_number":1224,"context_line":"        \"\"\""},{"line_number":1225,"context_line":"        encryption \u003d self._get_volume_encryption(context, connection_info,"},{"line_number":1226,"context_line":"                                                 encryption)"},{"line_number":1227,"context_line":"        if encryption:"},{"line_number":1228,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":1229,"context_line":"                                                   encryption)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_7221b69f","line":1226,"range":{"start_line":1226,"start_character":49,"end_line":1226,"end_character":59},"updated":"2017-12-21 00:09:15.000000000","message":"Same.","commit_id":"7d7c0131a5c3a9819a8cfc875bded71056eabb13"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"736a984132dfc5887fb90f3acc0004e2acc74f5f","unresolved":false,"context_lines":[{"line_number":1431,"context_line":"            # NOTE(lyarwood): We can provide None as the request context here"},{"line_number":1432,"context_line":"            # as we already have the encryption metadata dict from the compute"},{"line_number":1433,"context_line":"            # layer. This avoids the need to add the request context to the"},{"line_number":1434,"context_line":"            # signature of detach_volume requiring changes across all drivers."},{"line_number":1435,"context_line":"            self._detach_encryptor(None, connection_info, encryption)"},{"line_number":1436,"context_line":""},{"line_number":1437,"context_line":"        except exception.InstanceNotFound:"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf8cb3f7_12470a4e","line":1434,"updated":"2017-12-21 00:09:15.000000000","message":"Good comment.","commit_id":"7d7c0131a5c3a9819a8cfc875bded71056eabb13"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0c8b4f5665db8f56562f30ea170c237896d2f845","unresolved":false,"context_lines":[{"line_number":1190,"context_line":"                                               connection_info\u003dconnection_info,"},{"line_number":1191,"context_line":"                                               **encryption)"},{"line_number":1192,"context_line":""},{"line_number":1193,"context_line":"    def _get_volume_encryption(self, context, connection_info, encryption):"},{"line_number":1194,"context_line":"        \"\"\"Get the encryption metadata dict if it is not provided"},{"line_number":1195,"context_line":"        \"\"\""},{"line_number":1196,"context_line":"        if encryption is None and (\u0027data\u0027 in connection_info and"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf8cb3f7_72ae8b2f","line":1193,"range":{"start_line":1193,"start_character":63,"end_line":1193,"end_character":73},"updated":"2017-12-21 12:16:22.000000000","message":"You can drop this now - the None check is handled elsewhere and that\u0027s all we use this for here","commit_id":"eb2d3b95ea0380195aafc2c0207c09ca7682c7dd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0c8b4f5665db8f56562f30ea170c237896d2f845","unresolved":false,"context_lines":[{"line_number":1193,"context_line":"    def _get_volume_encryption(self, context, connection_info, encryption):"},{"line_number":1194,"context_line":"        \"\"\"Get the encryption metadata dict if it is not provided"},{"line_number":1195,"context_line":"        \"\"\""},{"line_number":1196,"context_line":"        if encryption is None and (\u0027data\u0027 in connection_info and"},{"line_number":1197,"context_line":"                \u0027volume_id\u0027 in connection_info[\u0027data\u0027]):"},{"line_number":1198,"context_line":"            volume_id \u003d connection_info[\u0027data\u0027][\u0027volume_id\u0027]"},{"line_number":1199,"context_line":"            encryption \u003d encryptors.get_encryption_metadata(context,"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf8cb3f7_32a8031e","line":1196,"range":{"start_line":1196,"start_character":11,"end_line":1196,"end_character":33},"updated":"2017-12-21 12:16:22.000000000","message":"Kill it. Kill it with fire.","commit_id":"eb2d3b95ea0380195aafc2c0207c09ca7682c7dd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7d01beaee3615843cbf68b60f3496dc877aefd83","unresolved":false,"context_lines":[{"line_number":1194,"context_line":"        \"\"\"Get the encryption metadata dict if it is not provided"},{"line_number":1195,"context_line":"        \"\"\""},{"line_number":1196,"context_line":"        encryption \u003d {}"},{"line_number":1197,"context_line":"        if \u0027data\u0027 in connection_info and \\"},{"line_number":1198,"context_line":"                \u0027volume_id\u0027 in connection_info[\u0027data\u0027]:"},{"line_number":1199,"context_line":"            volume_id \u003d connection_info[\u0027data\u0027][\u0027volume_id\u0027]"},{"line_number":1200,"context_line":"            encryption \u003d encryptors.get_encryption_metadata(context,"},{"line_number":1201,"context_line":"                            self._volume_api, volume_id, connection_info)"}],"source_content_type":"text/x-python","patch_set":10,"id":"bf8cb3f7_cd36dcb5","line":1198,"range":{"start_line":1197,"start_character":0,"end_line":1198,"end_character":55},"updated":"2017-12-21 12:33:31.000000000","message":"If you\u0027ve to respin, I guess this is equivalent to:\n\n  if connection_info.get(\u0027data\u0027, {}).get(\u0027data\u0027, None):","commit_id":"3bddb043f762a1356732c85ab84e9bc60de089da"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"355d1fa734491c6c838f935b875a4b1cd683a098","unresolved":false,"context_lines":[{"line_number":1395,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":1396,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":1397,"context_line":""},{"line_number":1398,"context_line":"            # The volume must be detached from the VM before disconnecting it"},{"line_number":1399,"context_line":"            # from its encryptor. Otherwise, the encryptor may report that the"},{"line_number":1400,"context_line":"            # volume is still in use."},{"line_number":1401,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(guest.get_disk,"},{"line_number":1402,"context_line":"                                                             disk_dev,"},{"line_number":1403,"context_line":"                                                             live\u003dlive)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_354fdb93","side":"PARENT","line":1400,"range":{"start_line":1398,"start_character":0,"end_line":1400,"end_character":37},"updated":"2018-01-10 11:51:23.000000000","message":"This comment still seems relevant. This seems like the sort of thing where at some point in the future somebody is going to be looking at that wait_for_detach() call and wondering if they can move it for some reason, or delete it entirely. Without this comment they\u0027re going to have to go git spelunking to know exactly why they shouldn\u0027t do that.\n\nI like this comment. It helps me understand the code, and it may save me a lot of time one day.","commit_id":"74deea4d8f66a85e66ec79c72c9f257f562d5afd"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"e740f351382ab4c6a0362a960ff1ed2623898585","unresolved":false,"context_lines":[{"line_number":1395,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":1396,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":1397,"context_line":""},{"line_number":1398,"context_line":"            # The volume must be detached from the VM before disconnecting it"},{"line_number":1399,"context_line":"            # from its encryptor. Otherwise, the encryptor may report that the"},{"line_number":1400,"context_line":"            # volume is still in use."},{"line_number":1401,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(guest.get_disk,"},{"line_number":1402,"context_line":"                                                             disk_dev,"},{"line_number":1403,"context_line":"                                                             live\u003dlive)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_7c222bbb","side":"PARENT","line":1400,"range":{"start_line":1398,"start_character":0,"end_line":1400,"end_character":37},"in_reply_to":"9f91af0f_354fdb93","updated":"2018-01-10 12:31:07.000000000","message":"I disagree, it\u0027s pretty obvious that we can\u0027t disconnect a volume (that now also detaches a connected encryptor) until the volume is detached from the guest. I don\u0027t think this comment adds any value in understanding why wait_for_detach has to come before _disconnect_volume IMHO.\n\nFWIW any change in the order here would break test_detach_volume_order_with_encryptors introduced in Ia0f8e725ec8a0fbc44bd4592b021dea978cf4e4f","commit_id":"74deea4d8f66a85e66ec79c72c9f257f562d5afd"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"8b02fa72cbde0021c319b249ddf4a63cbe50a440","unresolved":false,"context_lines":[{"line_number":1395,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":1396,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":1397,"context_line":""},{"line_number":1398,"context_line":"            # The volume must be detached from the VM before disconnecting it"},{"line_number":1399,"context_line":"            # from its encryptor. Otherwise, the encryptor may report that the"},{"line_number":1400,"context_line":"            # volume is still in use."},{"line_number":1401,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(guest.get_disk,"},{"line_number":1402,"context_line":"                                                             disk_dev,"},{"line_number":1403,"context_line":"                                                             live\u003dlive)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_dfdfa90a","side":"PARENT","line":1400,"range":{"start_line":1398,"start_character":0,"end_line":1400,"end_character":37},"in_reply_to":"9f91af0f_3fb2554a","updated":"2018-01-10 12:55:49.000000000","message":"\u003e Commenting as someone who\u0027s not terribly familiar with all the\n \u003e volume encryption internals, I agree with Matt—it helps a little\n \u003e bit to retain this comment.\n\nHow does it help when the call to encryptor.detach_volume has been removed?\n\n \u003e It appears like you\u0027re going out of your way to remove an existing\n \u003e comment, which some developers find it useful.  What is obvious to\n \u003e you can be a far cry from it for a fresh pair of eyes trying to get\n \u003e up-to-speed.  It isn\u0027t confusing anyone, and the comment helps\n \u003e (even if not to the fullest extent) as a reminder, please leave it\n \u003e in.\n\nI\u0027m going out of my way to remove a confusing comment now that the call it was referring to has been hidden in _disconnect_volume.","commit_id":"74deea4d8f66a85e66ec79c72c9f257f562d5afd"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"33b2bb067946a721c3e1e90f4b30fa8bdc62d0a1","unresolved":false,"context_lines":[{"line_number":1395,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":1396,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":1397,"context_line":""},{"line_number":1398,"context_line":"            # The volume must be detached from the VM before disconnecting it"},{"line_number":1399,"context_line":"            # from its encryptor. Otherwise, the encryptor may report that the"},{"line_number":1400,"context_line":"            # volume is still in use."},{"line_number":1401,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(guest.get_disk,"},{"line_number":1402,"context_line":"                                                             disk_dev,"},{"line_number":1403,"context_line":"                                                             live\u003dlive)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_3fb2554a","side":"PARENT","line":1400,"range":{"start_line":1398,"start_character":0,"end_line":1400,"end_character":37},"in_reply_to":"9f91af0f_7c222bbb","updated":"2018-01-10 12:42:02.000000000","message":"Commenting as someone who\u0027s not terribly familiar with all the volume encryption internals, I agree with Matt—it helps a little bit to retain this comment.\n\nIt appears like you\u0027re going out of your way to remove an existing comment, which some developers find it useful.  What is obvious to you can be a far cry from it for a fresh pair of eyes trying to get up-to-speed.  It isn\u0027t confusing anyone, and the comment helps (even if not to the fullest extent) as a reminder, please leave it in.","commit_id":"74deea4d8f66a85e66ec79c72c9f257f562d5afd"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"4b08c04fa0cc81879a86830db08753c41dc0ce3b","unresolved":false,"context_lines":[{"line_number":1395,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":1396,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":1397,"context_line":""},{"line_number":1398,"context_line":"            # The volume must be detached from the VM before disconnecting it"},{"line_number":1399,"context_line":"            # from its encryptor. Otherwise, the encryptor may report that the"},{"line_number":1400,"context_line":"            # volume is still in use."},{"line_number":1401,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(guest.get_disk,"},{"line_number":1402,"context_line":"                                                             disk_dev,"},{"line_number":1403,"context_line":"                                                             live\u003dlive)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_1f4751be","side":"PARENT","line":1400,"range":{"start_line":1398,"start_character":0,"end_line":1400,"end_character":37},"in_reply_to":"9f91af0f_dfdfa90a","updated":"2018-01-10 13:00:30.000000000","message":"I\u0027d argue that the comment is even more important now that the effect it\u0027s guarding against is further away.\n\nAlso, the test doesn\u0027t explain *why*. I\u0027ve hit this many times tweaking stuff in this driver where tests assert things, and you can\u0027t tell if they\u0027re just asserting it because that\u0027s how it is, or because that\u0027s how it has to be. i.e. It doesn\u0027t help me know if I can safely change it.","commit_id":"74deea4d8f66a85e66ec79c72c9f257f562d5afd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3bc4c957ce7e84d2a56e5a8d99c3f254bff0f613","unresolved":false,"context_lines":[{"line_number":1193,"context_line":"        \"\"\"Get the encryption metadata dict if it is not provided"},{"line_number":1194,"context_line":"        \"\"\""},{"line_number":1195,"context_line":"        encryption \u003d {}"},{"line_number":1196,"context_line":"        if \u0027data\u0027 in connection_info and \\"},{"line_number":1197,"context_line":"                \u0027volume_id\u0027 in connection_info[\u0027data\u0027]:"},{"line_number":1198,"context_line":"            volume_id \u003d connection_info[\u0027data\u0027][\u0027volume_id\u0027]"},{"line_number":1199,"context_line":"            encryption \u003d encryptors.get_encryption_metadata(context,"},{"line_number":1200,"context_line":"                            self._volume_api, volume_id, connection_info)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_952fd93d","line":1197,"range":{"start_line":1196,"start_character":0,"end_line":1197,"end_character":55},"updated":"2018-01-10 10:28:44.000000000","message":"Same comment as before:\n\n  if connection_info.get(\u0027data\u0027, {}).get(\u0027volume_id\u0027):\n\nBut this is as above and a nit so meh","commit_id":"169a238530eb46eb3eb3b214abf4b90a64584239"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"355d1fa734491c6c838f935b875a4b1cd683a098","unresolved":false,"context_lines":[{"line_number":1193,"context_line":"        \"\"\"Get the encryption metadata dict if it is not provided"},{"line_number":1194,"context_line":"        \"\"\""},{"line_number":1195,"context_line":"        encryption \u003d {}"},{"line_number":1196,"context_line":"        if \u0027data\u0027 in connection_info and \\"},{"line_number":1197,"context_line":"                \u0027volume_id\u0027 in connection_info[\u0027data\u0027]:"},{"line_number":1198,"context_line":"            volume_id \u003d connection_info[\u0027data\u0027][\u0027volume_id\u0027]"},{"line_number":1199,"context_line":"            encryption \u003d encryptors.get_encryption_metadata(context,"},{"line_number":1200,"context_line":"                            self._volume_api, volume_id, connection_info)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_75aa4319","line":1197,"range":{"start_line":1196,"start_character":0,"end_line":1197,"end_character":55},"in_reply_to":"9f91af0f_952fd93d","updated":"2018-01-10 11:51:23.000000000","message":"Actually I\u0027d prefer if we stopped looking in connection_info at all, and returned volume_id from the BDM which we will always have in context somewhere further up the call stack.\n\nBut also: meh. This is fine for now.","commit_id":"169a238530eb46eb3eb3b214abf4b90a64584239"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3bc4c957ce7e84d2a56e5a8d99c3f254bff0f613","unresolved":false,"context_lines":[{"line_number":1270,"context_line":"                        \"block size\") % CONF.libvirt.virt_type"},{"line_number":1271,"context_line":"                raise exception.InvalidHypervisorType(msg)"},{"line_number":1272,"context_line":""},{"line_number":1273,"context_line":"        self._connect_volume(context, connection_info, instance,"},{"line_number":1274,"context_line":"                             encryption\u003dencryption)"},{"line_number":1275,"context_line":"        disk_info \u003d blockinfo.get_info_from_bdm("},{"line_number":1276,"context_line":"            instance, CONF.libvirt.virt_type, instance.image_meta, bdm)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_55a7ffbd","line":1273,"range":{"start_line":1273,"start_character":29,"end_line":1273,"end_character":36},"updated":"2018-01-10 10:28:44.000000000","message":"Could we not pass None in here too, as with the call to \u0027_disconnect_volume\u0027 in \u0027detach_volume\u0027, given that we have the encryption parameter available to us? Would make this a little easier to reason about, if so.","commit_id":"169a238530eb46eb3eb3b214abf4b90a64584239"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"fd516a5b742f2a8efdd8b8539681b5b3922b400c","unresolved":false,"context_lines":[{"line_number":1270,"context_line":"                        \"block size\") % CONF.libvirt.virt_type"},{"line_number":1271,"context_line":"                raise exception.InvalidHypervisorType(msg)"},{"line_number":1272,"context_line":""},{"line_number":1273,"context_line":"        self._connect_volume(context, connection_info, instance,"},{"line_number":1274,"context_line":"                             encryption\u003dencryption)"},{"line_number":1275,"context_line":"        disk_info \u003d blockinfo.get_info_from_bdm("},{"line_number":1276,"context_line":"            instance, CONF.libvirt.virt_type, instance.image_meta, bdm)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_b5fc4b70","line":1273,"range":{"start_line":1273,"start_character":29,"end_line":1273,"end_character":36},"in_reply_to":"9f91af0f_55a7ffbd","updated":"2018-01-10 10:36:13.000000000","message":"Looks like we need context either way because it\u0027s passed to encryptor.attach_volume().","commit_id":"169a238530eb46eb3eb3b214abf4b90a64584239"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a74c8ee80c8f1bf88840eecb64e5eb291834f65b","unresolved":false,"context_lines":[{"line_number":1270,"context_line":"                        \"block size\") % CONF.libvirt.virt_type"},{"line_number":1271,"context_line":"                raise exception.InvalidHypervisorType(msg)"},{"line_number":1272,"context_line":""},{"line_number":1273,"context_line":"        self._connect_volume(context, connection_info, instance,"},{"line_number":1274,"context_line":"                             encryption\u003dencryption)"},{"line_number":1275,"context_line":"        disk_info \u003d blockinfo.get_info_from_bdm("},{"line_number":1276,"context_line":"            instance, CONF.libvirt.virt_type, instance.image_meta, bdm)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_155e5775","line":1273,"range":{"start_line":1273,"start_character":29,"end_line":1273,"end_character":36},"in_reply_to":"9f91af0f_b5fc4b70","updated":"2018-01-10 10:38:23.000000000","message":"Right you are. I\u0027d missed that.","commit_id":"169a238530eb46eb3eb3b214abf4b90a64584239"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"355d1fa734491c6c838f935b875a4b1cd683a098","unresolved":false,"context_lines":[{"line_number":1444,"context_line":"            else:"},{"line_number":1445,"context_line":"                raise"},{"line_number":1446,"context_line":""},{"line_number":1447,"context_line":"        # NOTE(lyarwood): We can provide None as the request context here as we"},{"line_number":1448,"context_line":"        # already have the encryption metadata dict from the compute layer."},{"line_number":1449,"context_line":"        # This avoids the need to add the request context to the signature of"},{"line_number":1450,"context_line":"        # detach_volume requiring changes across all drivers."},{"line_number":1451,"context_line":"        self._disconnect_volume(None, connection_info, instance,"},{"line_number":1452,"context_line":"                                encryption\u003dencryption)"},{"line_number":1453,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9f91af0f_55529f3c","line":1450,"range":{"start_line":1447,"start_character":0,"end_line":1450,"end_character":61},"updated":"2018-01-10 11:51:23.000000000","message":"Nice.","commit_id":"169a238530eb46eb3eb3b214abf4b90a64584239"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"479cea08d576ae2515734781f89f6b631a028c6a","unresolved":false,"context_lines":[{"line_number":1421,"context_line":"            # NOTE(lyarwood): The volume must be detached from the VM before"},{"line_number":1422,"context_line":"            # detaching any attached encryptors or disconnecting the underlying"},{"line_number":1423,"context_line":"            # volume in _disconnect_volume. Otherwise, the encryptor or volume"},{"line_number":1424,"context_line":"            # driver may report that the volume is still in use."},{"line_number":1425,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(guest.get_disk,"},{"line_number":1426,"context_line":"                                                             disk_dev,"},{"line_number":1427,"context_line":"                                                             live\u003dlive)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f91af0f_ed30c758","line":1424,"updated":"2018-01-10 15:32:59.000000000","message":"Thanks :)","commit_id":"415f30c58d4d1d0c4a9c6c481a27d9a4964f810d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5deda0ea9cc2d3c330b568dad563faaaf0c73ec6","unresolved":false,"context_lines":[{"line_number":5248,"context_line":"                if encryption:"},{"line_number":5249,"context_line":"                    encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":5250,"context_line":"                                                           encryption)"},{"line_number":5251,"context_line":"                    encryptor.attach_volume(context, **encryption)"},{"line_number":5252,"context_line":""},{"line_number":5253,"context_line":"        timeout \u003d CONF.vif_plugging_timeout"},{"line_number":5254,"context_line":"        if (self._conn_supports_start_paused and"}],"source_content_type":"text/x-python","patch_set":16,"id":"7f96bb07_61e53338","side":"PARENT","line":5251,"updated":"2018-01-22 16:46:52.000000000","message":"so this is being removed because this is done in _connect_volume() now and that is executed before _create_domain_and_network()?","commit_id":"0ff7af38ffebc55a8b2842cee798d02eae7f3ea5"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"f7fe433225347f655a039ba1e9dc5ea17cb54d2c","unresolved":false,"context_lines":[{"line_number":5248,"context_line":"                if encryption:"},{"line_number":5249,"context_line":"                    encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":5250,"context_line":"                                                           encryption)"},{"line_number":5251,"context_line":"                    encryptor.attach_volume(context, **encryption)"},{"line_number":5252,"context_line":""},{"line_number":5253,"context_line":"        timeout \u003d CONF.vif_plugging_timeout"},{"line_number":5254,"context_line":"        if (self._conn_supports_start_paused and"}],"source_content_type":"text/x-python","patch_set":16,"id":"7f96bb07_a1933b5c","side":"PARENT","line":5251,"in_reply_to":"7f96bb07_61e53338","updated":"2018-01-22 16:53:49.000000000","message":"Correct, _get_guest_xml calls down into _get_guest_storage_config that eventually calls _connect_volume where we\u0027ve moved these calls to attach an encryptor if required. \n\nWe did look at moving _connect_volume further up the chain but it made the size of the refactor balloon massively. Something to look at again for R.","commit_id":"0ff7af38ffebc55a8b2842cee798d02eae7f3ea5"}]}