)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"0a5ae673454ab300dc4a8dedab482bf7bd4a8a0c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Attach/detach encryptors during swap_volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When migrating a in-use encrypted volume, it should"},{"line_number":10,"context_line":"attach encryptor of new volume before rebasing. And detach"},{"line_number":11,"context_line":"encryptor of old volume before disconnecting the volume."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a7e1126_a470eb0b","line":9,"range":{"start_line":9,"start_character":15,"end_line":9,"end_character":16},"updated":"2015-12-22 07:27:29.000000000","message":"an","commit_id":"a0fde9d32d8720cc8af9768297d8fc70622ddf9e"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"ebcfc873faf159e6fe46f8e13f472a617d6b3521","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Attach/detach encryptors during swap_volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When migrating a in-use encrypted volume, it should"},{"line_number":10,"context_line":"attach encryptor of new volume before rebasing. And detach"},{"line_number":11,"context_line":"encryptor of old volume before disconnecting the volume."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa69d971_db8129fd","line":9,"range":{"start_line":9,"start_character":15,"end_line":9,"end_character":16},"in_reply_to":"3a7e1126_a470eb0b","updated":"2015-12-30 08:47:49.000000000","message":"Done","commit_id":"a0fde9d32d8720cc8af9768297d8fc70622ddf9e"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"77a3173efc3bdac0b17d2bb68c61932d78c1ec54","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Attach/detach encryptors during swap_volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When migrating an in-use encrypted volume, it should"},{"line_number":10,"context_line":"attach encryptor of new volume before rebasing. And detach"},{"line_number":11,"context_line":"encryptor of old volume before disconnecting the volume."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"ba5da102_626475b8","line":9,"updated":"2016-11-03 11:31:20.000000000","message":"nit - s/migrating/swapping/g","commit_id":"0a4f13ed9fb6fcc822c2a7d35af32ddcf19d3c94"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"d79714406bb9ba3b09e88ea783814a6a48430b33","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Attach/detach encryptors during swap_volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When migrating an in-use encrypted volume, it should"},{"line_number":10,"context_line":"attach encryptor of new volume before rebasing. And detach"},{"line_number":11,"context_line":"encryptor of old volume before disconnecting the volume."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"ba5da102_feff2063","line":9,"in_reply_to":"ba5da102_626475b8","updated":"2016-11-04 00:52:52.000000000","message":"Done","commit_id":"0a4f13ed9fb6fcc822c2a7d35af32ddcf19d3c94"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"77a3173efc3bdac0b17d2bb68c61932d78c1ec54","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Attach/detach encryptors during swap_volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When migrating an in-use encrypted volume, it should"},{"line_number":10,"context_line":"attach encryptor of new volume before rebasing. And detach"},{"line_number":11,"context_line":"encryptor of old volume before disconnecting the volume."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch is to add such operations in swap_volume."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"ba5da102_8257c9c8","line":10,"updated":"2016-11-03 11:31:20.000000000","message":"nit - remove the s/\\.\\ And/\\ and/g","commit_id":"0a4f13ed9fb6fcc822c2a7d35af32ddcf19d3c94"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"d79714406bb9ba3b09e88ea783814a6a48430b33","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Attach/detach encryptors during swap_volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When migrating an in-use encrypted volume, it should"},{"line_number":10,"context_line":"attach encryptor of new volume before rebasing. And detach"},{"line_number":11,"context_line":"encryptor of old volume before disconnecting the volume."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch is to add such operations in swap_volume."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"ba5da102_1efbf479","line":10,"in_reply_to":"ba5da102_8257c9c8","updated":"2016-11-04 00:52:52.000000000","message":"Done","commit_id":"0a4f13ed9fb6fcc822c2a7d35af32ddcf19d3c94"}],"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"09d4332933310585b085e6e2d19803c7568f0f91","unresolved":false,"context_lines":[{"line_number":4739,"context_line":"                                                                connector,"},{"line_number":4740,"context_line":"                                                                instance,"},{"line_number":4741,"context_line":"                                                                bdm)"},{"line_number":4742,"context_line":"            # Get encryption"},{"line_number":4743,"context_line":"            old_encryption \u003d encryptors.get_encryption_metadata("},{"line_number":4744,"context_line":"                                 context, self.volume_api,"},{"line_number":4745,"context_line":"                                 old_volume_id, old_cinfo)"},{"line_number":4746,"context_line":"            new_encryption \u003d encryptors.get_encryption_metadata("},{"line_number":4747,"context_line":"                                 context, self.volume_api,"},{"line_number":4748,"context_line":"                                 new_volume_id, new_cinfo)"},{"line_number":4749,"context_line":""},{"line_number":4750,"context_line":"            LOG.debug(\"swap_volume: Calling driver volume swap with \""},{"line_number":4751,"context_line":"                      \"connection infos: new: %(new_cinfo)s; \""}],"source_content_type":"text/x-python","patch_set":8,"id":"9abb7d3a_bc0af623","line":4748,"range":{"start_line":4742,"start_character":12,"end_line":4748,"end_character":58},"updated":"2016-06-01 16:51:06.000000000","message":"We could technically do this in the libvirt driver\u0027s swap_volume method itself, but it\u0027s maybe best to do it in the compute manager and pass the information down. The reason I bring this up is then we wouldn\u0027t need to change the compute manager or the virt driver interface and it would make this change smaller and more focused.","commit_id":"1bec009d6c0dc873707d77b928fc6563d59363d9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0b9ca1b5a87368f763821b2832e8664c623c97df","unresolved":false,"context_lines":[{"line_number":4739,"context_line":"                                                                connector,"},{"line_number":4740,"context_line":"                                                                instance,"},{"line_number":4741,"context_line":"                                                                bdm)"},{"line_number":4742,"context_line":"            # Get encryption"},{"line_number":4743,"context_line":"            old_encryption \u003d encryptors.get_encryption_metadata("},{"line_number":4744,"context_line":"                                 context, self.volume_api,"},{"line_number":4745,"context_line":"                                 old_volume_id, old_cinfo)"},{"line_number":4746,"context_line":"            new_encryption \u003d encryptors.get_encryption_metadata("},{"line_number":4747,"context_line":"                                 context, self.volume_api,"},{"line_number":4748,"context_line":"                                 new_volume_id, new_cinfo)"},{"line_number":4749,"context_line":""},{"line_number":4750,"context_line":"            LOG.debug(\"swap_volume: Calling driver volume swap with \""},{"line_number":4751,"context_line":"                      \"connection infos: new: %(new_cinfo)s; \""}],"source_content_type":"text/x-python","patch_set":8,"id":"9abb7d3a_5d3455dd","line":4748,"range":{"start_line":4742,"start_character":12,"end_line":4748,"end_character":58},"in_reply_to":"9abb7d3a_4e7a18be","updated":"2016-06-02 21:06:37.000000000","message":"There are no other virt drivers that support swap volume, so meh.","commit_id":"1bec009d6c0dc873707d77b928fc6563d59363d9"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"181e6cae09198a1d0fa683427706110651c2d280","unresolved":false,"context_lines":[{"line_number":4739,"context_line":"                                                                connector,"},{"line_number":4740,"context_line":"                                                                instance,"},{"line_number":4741,"context_line":"                                                                bdm)"},{"line_number":4742,"context_line":"            # Get encryption"},{"line_number":4743,"context_line":"            old_encryption \u003d encryptors.get_encryption_metadata("},{"line_number":4744,"context_line":"                                 context, self.volume_api,"},{"line_number":4745,"context_line":"                                 old_volume_id, old_cinfo)"},{"line_number":4746,"context_line":"            new_encryption \u003d encryptors.get_encryption_metadata("},{"line_number":4747,"context_line":"                                 context, self.volume_api,"},{"line_number":4748,"context_line":"                                 new_volume_id, new_cinfo)"},{"line_number":4749,"context_line":""},{"line_number":4750,"context_line":"            LOG.debug(\"swap_volume: Calling driver volume swap with \""},{"line_number":4751,"context_line":"                      \"connection infos: new: %(new_cinfo)s; \""}],"source_content_type":"text/x-python","patch_set":8,"id":"9abb7d3a_270fb2e2","line":4748,"range":{"start_line":4742,"start_character":12,"end_line":4748,"end_character":58},"in_reply_to":"9abb7d3a_5d3455dd","updated":"2016-06-03 05:32:38.000000000","message":"I think it is better to keep the framework. Meanwhile, as new parameter context is needed, the interface swap_volume needs to change no matter encryption is got here or later in driver. @Matt, if you insist, I can make the change.","commit_id":"1bec009d6c0dc873707d77b928fc6563d59363d9"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"61778301d7c402cb0cbd507184c1f8264de64c09","unresolved":false,"context_lines":[{"line_number":4739,"context_line":"                                                                connector,"},{"line_number":4740,"context_line":"                                                                instance,"},{"line_number":4741,"context_line":"                                                                bdm)"},{"line_number":4742,"context_line":"            # Get encryption"},{"line_number":4743,"context_line":"            old_encryption \u003d encryptors.get_encryption_metadata("},{"line_number":4744,"context_line":"                                 context, self.volume_api,"},{"line_number":4745,"context_line":"                                 old_volume_id, old_cinfo)"},{"line_number":4746,"context_line":"            new_encryption \u003d encryptors.get_encryption_metadata("},{"line_number":4747,"context_line":"                                 context, self.volume_api,"},{"line_number":4748,"context_line":"                                 new_volume_id, new_cinfo)"},{"line_number":4749,"context_line":""},{"line_number":4750,"context_line":"            LOG.debug(\"swap_volume: Calling driver volume swap with \""},{"line_number":4751,"context_line":"                      \"connection infos: new: %(new_cinfo)s; \""}],"source_content_type":"text/x-python","patch_set":8,"id":"9abb7d3a_4e7a18be","line":4748,"range":{"start_line":4742,"start_character":12,"end_line":4748,"end_character":58},"in_reply_to":"9abb7d3a_bc0af623","updated":"2016-06-02 03:52:08.000000000","message":"In my opinion, to get encryption is alike getting connection_info, no need to duplicate the codes in every driver. \n\nI hope I added enough unit test.","commit_id":"1bec009d6c0dc873707d77b928fc6563d59363d9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"09d4332933310585b085e6e2d19803c7568f0f91","unresolved":false,"context_lines":[{"line_number":4752,"context_line":"                      \"old: %(old_cinfo)s and encryption infos: \""},{"line_number":4753,"context_line":"                      \"new: %(new_enc)s; old: %(old_enc)s.\","},{"line_number":4754,"context_line":"                      {\u0027new_cinfo\u0027: new_cinfo, \u0027old_cinfo\u0027: old_cinfo,"},{"line_number":4755,"context_line":"                       \u0027new_enc\u0027: new_encryption, \u0027old_enc\u0027: old_encryption},"},{"line_number":4756,"context_line":"                      contex\u003dcontext, instance\u003dinstance)"},{"line_number":4757,"context_line":"            self.driver.swap_volume(context, old_cinfo, new_cinfo,"},{"line_number":4758,"context_line":"                                    old_encryption, new_encryption,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9abb7d3a_bc6b161d","line":4755,"range":{"start_line":4755,"start_character":23,"end_line":4755,"end_character":75},"updated":"2016-06-01 16:51:06.000000000","message":"Are these safe to log without scrubbing for passwords first?\n\nLooks like they aren\u0027t:\n\nhttps://github.com/openstack/nova/blob/master/nova/volume/encryptors/__init__.py#L83","commit_id":"1bec009d6c0dc873707d77b928fc6563d59363d9"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"61778301d7c402cb0cbd507184c1f8264de64c09","unresolved":false,"context_lines":[{"line_number":4752,"context_line":"                      \"old: %(old_cinfo)s and encryption infos: \""},{"line_number":4753,"context_line":"                      \"new: %(new_enc)s; old: %(old_enc)s.\","},{"line_number":4754,"context_line":"                      {\u0027new_cinfo\u0027: new_cinfo, \u0027old_cinfo\u0027: old_cinfo,"},{"line_number":4755,"context_line":"                       \u0027new_enc\u0027: new_encryption, \u0027old_enc\u0027: old_encryption},"},{"line_number":4756,"context_line":"                      contex\u003dcontext, instance\u003dinstance)"},{"line_number":4757,"context_line":"            self.driver.swap_volume(context, old_cinfo, new_cinfo,"},{"line_number":4758,"context_line":"                                    old_encryption, new_encryption,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9abb7d3a_1ad04bc8","line":4755,"range":{"start_line":4755,"start_character":23,"end_line":4755,"end_character":75},"in_reply_to":"9abb7d3a_bc6b161d","updated":"2016-06-02 03:52:08.000000000","message":"I should mask the key. Done.","commit_id":"1bec009d6c0dc873707d77b928fc6563d59363d9"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"561019f490dc9720cf5bbee81822f6a17c051cb8","unresolved":false,"context_lines":[{"line_number":609,"context_line":"        self.flags(snapshots_directory\u003dtemp_dir, group\u003d\u0027libvirt\u0027)"},{"line_number":610,"context_line":"        self.flags(fixed_key\u003d\u002700000000000000000000000000000000\u0027"},{"line_number":611,"context_line":"                             \u002700000000000000000000000000000000\u0027,"},{"line_number":612,"context_line":"                   group\u003d\u0027keymgr\u0027)"},{"line_number":613,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":614,"context_line":"            \u0027nova.virt.libvirt.driver.libvirt_utils\u0027,"},{"line_number":615,"context_line":"            fake_libvirt_utils))"}],"source_content_type":"text/x-python","patch_set":7,"id":"1af94dfe_9807d21b","line":612,"updated":"2016-03-16 18:50:51.000000000","message":"I don\u0027t think it\u0027s good to hard-code the configuration-based key manager into this test class. Better to mock it for the specific tests that require it and use the existing fake_api -- e.g.,\n\n  @mock.patch.object(keymgr, \u0027API\u0027, fake_keymgr.fake_api)","commit_id":"62ed78719b6edbf36e52279f350158034d5318fe"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"90025d0607f064b651850f56a32806159f2bb2d9","unresolved":false,"context_lines":[{"line_number":609,"context_line":"        self.flags(snapshots_directory\u003dtemp_dir, group\u003d\u0027libvirt\u0027)"},{"line_number":610,"context_line":"        self.flags(fixed_key\u003d\u002700000000000000000000000000000000\u0027"},{"line_number":611,"context_line":"                             \u002700000000000000000000000000000000\u0027,"},{"line_number":612,"context_line":"                   group\u003d\u0027keymgr\u0027)"},{"line_number":613,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":614,"context_line":"            \u0027nova.virt.libvirt.driver.libvirt_utils\u0027,"},{"line_number":615,"context_line":"            fake_libvirt_utils))"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_6c4a0829","line":612,"in_reply_to":"1af94dfe_9807d21b","updated":"2016-05-04 06:16:35.000000000","message":"Done","commit_id":"62ed78719b6edbf36e52279f350158034d5318fe"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"561019f490dc9720cf5bbee81822f6a17c051cb8","unresolved":false,"context_lines":[{"line_number":13303,"context_line":"                         instance, \u0027/dev/vdb\u0027, 1)"},{"line_number":13304,"context_line":""},{"line_number":13305,"context_line":"        attach_enc.assert_called_once_with(self.context, **new_enc)"},{"line_number":13306,"context_line":"        detach_enc.assert_called_once_with(**old_enc)"},{"line_number":13307,"context_line":"        get_guest.assert_called_once_with(instance)"},{"line_number":13308,"context_line":"        connect_volume.assert_called_once_with(new_connection_info, disk_info)"},{"line_number":13309,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1af94dfe_33222102","line":13306,"updated":"2016-03-16 18:50:51.000000000","message":"Following up on the comments on test_virt_drivers, why include these assertions as part of this test case? It seems like they should be part of test_swap_encrypted_volume.","commit_id":"62ed78719b6edbf36e52279f350158034d5318fe"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"90025d0607f064b651850f56a32806159f2bb2d9","unresolved":false,"context_lines":[{"line_number":13303,"context_line":"                         instance, \u0027/dev/vdb\u0027, 1)"},{"line_number":13304,"context_line":""},{"line_number":13305,"context_line":"        attach_enc.assert_called_once_with(self.context, **new_enc)"},{"line_number":13306,"context_line":"        detach_enc.assert_called_once_with(**old_enc)"},{"line_number":13307,"context_line":"        get_guest.assert_called_once_with(instance)"},{"line_number":13308,"context_line":"        connect_volume.assert_called_once_with(new_connection_info, disk_info)"},{"line_number":13309,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_2c98106a","line":13306,"in_reply_to":"1af94dfe_33222102","updated":"2016-05-04 06:16:35.000000000","message":"Hi Joel, please see the comments I replied to Dane.","commit_id":"62ed78719b6edbf36e52279f350158034d5318fe"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"57ea897d01d488c70c87c4f30cee12a98dfce858","unresolved":false,"context_lines":[{"line_number":14383,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver._connect_volume\u0027)"},{"line_number":14384,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":14385,"context_line":"    @mock.patch.object(keymgr, \u0027API\u0027, fake_keymgr.fake_api)"},{"line_number":14386,"context_line":"    def _test_swap_volume_driver_bdm_save(self,"},{"line_number":14387,"context_line":"                                          get_guest,"},{"line_number":14388,"context_line":"                                          connect_volume, get_volume_config,"},{"line_number":14389,"context_line":"                                          get_by_volume_and_instance,"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a629dbe_a296d13c","line":14386,"updated":"2016-11-10 03:19:29.000000000","message":"An argument should be added for line 14385(keymgr \u0027API\u0027).\nIt is missing.","commit_id":"1b9cd39841f927fa05a34fc2cc546f325438571c"}],"nova/tests/unit/virt/test_virt_drivers.py":[{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"3806807a3233967dd2849fca84b5fa283f8ed362","unresolved":false,"context_lines":[{"line_number":511,"context_line":"                                         \u0027data\u0027: {}},"},{"line_number":512,"context_line":"                                        None, None,"},{"line_number":513,"context_line":"                                        instance_ref,"},{"line_number":514,"context_line":"                                        \u0027/dev/sda\u0027, 2))"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"    @catch_notimplementederror"},{"line_number":517,"context_line":"    def test_attach_detach_different_power_states(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afc51ec_7213555d","line":514,"updated":"2016-03-11 14:42:01.000000000","message":"We need another test here which mocks out the new_encryption and old_encryption parameters and tests that the swap_volume method actually works as it\u0027s supposed to.","commit_id":"f010c489bd1a300b42755d5d69037c6f66723ab4"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"f2d7cd9b1f706d99da1bbffa8bd70e36a19347eb","unresolved":false,"context_lines":[{"line_number":511,"context_line":"                                         \u0027data\u0027: {}},"},{"line_number":512,"context_line":"                                        None, None,"},{"line_number":513,"context_line":"                                        instance_ref,"},{"line_number":514,"context_line":"                                        \u0027/dev/sda\u0027, 2))"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"    @catch_notimplementederror"},{"line_number":517,"context_line":"    def test_attach_detach_different_power_states(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3afc51ec_f27530fa","line":514,"in_reply_to":"3afc51ec_7213555d","updated":"2016-03-15 06:41:11.000000000","message":"Here it is simple test for fake_driver and libvirt driver, it does detailed test in nova/tests/unit/virt/libvirt/test_driver.py.\n\nAnyway, I added a new test.","commit_id":"f010c489bd1a300b42755d5d69037c6f66723ab4"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"fcd67ec2a6fef92d22a0a7c7a1d6fe9df15019d7","unresolved":false,"context_lines":[{"line_number":537,"context_line":"                                         \u0027data\u0027: {}},"},{"line_number":538,"context_line":"                                        old_enc, new_enc,"},{"line_number":539,"context_line":"                                        instance_ref,"},{"line_number":540,"context_line":"                                        \u0027/dev/sda\u0027, 2))"},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"    @catch_notimplementederror"},{"line_number":543,"context_line":"    def test_attach_detach_different_power_states(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1af94dfe_44749fce","line":540,"updated":"2016-03-15 13:46:25.000000000","message":"Thanks for adding this test, but it\u0027s still not complete. You need an assert before the swap call to check that the volume has the old encryption data, and an assert after the swap call to check that the volume has the new encryption data.\n\nAs this unit test is currently written, it doesn\u0027t prevent someone merging a change which eliminates the functionality you are implementing.","commit_id":"62ed78719b6edbf36e52279f350158034d5318fe"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"90025d0607f064b651850f56a32806159f2bb2d9","unresolved":false,"context_lines":[{"line_number":537,"context_line":"                                         \u0027data\u0027: {}},"},{"line_number":538,"context_line":"                                        old_enc, new_enc,"},{"line_number":539,"context_line":"                                        instance_ref,"},{"line_number":540,"context_line":"                                        \u0027/dev/sda\u0027, 2))"},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"    @catch_notimplementederror"},{"line_number":543,"context_line":"    def test_attach_detach_different_power_states(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_4ce10cfc","line":540,"in_reply_to":"1af94dfe_44749fce","updated":"2016-05-04 06:16:35.000000000","message":"Hi, Dane, sorry I have to remove the test function test_swap_encrypted_volume.\n\nThe test class _VirtDriverTestCase is to test both nova.virt.libvirt.LibvirtDriver and nova.virt.fake.FakeDriver.\n\nFakeDriver: https://github.com/openstack/nova/blob/master/nova/virt/fake.py#L123  \n\nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/virt/test_virt_drivers.py#L240\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/virt/test_virt_drivers.py#L840\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/virt/test_virt_drivers.py#L860\n\nThese tests in this class is just to verify the interface exists. I can\u0027t check the behavior of encrypted volume in this function as swap_volume in FakeDriver does nothing.\n\nMeanwhile the following test is to test driver in libvirt, and I check the attach/detach of the encrypted volume.\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_driver.py","commit_id":"62ed78719b6edbf36e52279f350158034d5318fe"}],"nova/virt/driver.py":[{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"0a5ae673454ab300dc4a8dedab482bf7bd4a8a0c","unresolved":false,"context_lines":[{"line_number":499,"context_line":"        raise NotImplementedError()"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"    def swap_volume(self, context, old_connection_info, new_connection_info,"},{"line_number":502,"context_line":"                    old_encryption, new_encryption,"},{"line_number":503,"context_line":"                    instance, mountpoint, resize_to):"},{"line_number":504,"context_line":"        \"\"\"Replace the volume attached to the given `instance`."},{"line_number":505,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_84292f9d","line":502,"range":{"start_line":502,"start_character":20,"end_line":502,"end_character":34},"updated":"2015-12-22 07:27:29.000000000","message":"you have update two new parameters, please update the doc string below","commit_id":"a0fde9d32d8720cc8af9768297d8fc70622ddf9e"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"ebcfc873faf159e6fe46f8e13f472a617d6b3521","unresolved":false,"context_lines":[{"line_number":499,"context_line":"        raise NotImplementedError()"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"    def swap_volume(self, context, old_connection_info, new_connection_info,"},{"line_number":502,"context_line":"                    old_encryption, new_encryption,"},{"line_number":503,"context_line":"                    instance, mountpoint, resize_to):"},{"line_number":504,"context_line":"        \"\"\"Replace the volume attached to the given `instance`."},{"line_number":505,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_7b17dded","line":502,"range":{"start_line":502,"start_character":20,"end_line":502,"end_character":34},"in_reply_to":"3a7e1126_84292f9d","updated":"2015-12-30 08:47:49.000000000","message":"Done","commit_id":"a0fde9d32d8720cc8af9768297d8fc70622ddf9e"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"0db13aa4a3174d95caaeb0eaca0e559cadc9de46","unresolved":false,"context_lines":[{"line_number":502,"context_line":"                    old_encryption, new_encryption,"},{"line_number":503,"context_line":"                    instance, mountpoint, resize_to):"},{"line_number":504,"context_line":"        \"\"\"Replace the volume attached to the given `instance`."},{"line_number":505,"context_line":""},{"line_number":506,"context_line":"        :param dict old_connection_info:"},{"line_number":507,"context_line":"            The volume for this connection gets detached from the given"},{"line_number":508,"context_line":"            `instance`."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba63e191_a14bcdaf","line":505,"updated":"2016-01-19 10:36:38.000000000","message":"A comment for \u0027context\u0027 should be added.","commit_id":"36b75c370dfbd0c3125e5b9e4056ca2afb7733ce"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"b2f78b8c6cf9617d5081dcb285788c185d19fd6c","unresolved":false,"context_lines":[{"line_number":502,"context_line":"                    old_encryption, new_encryption,"},{"line_number":503,"context_line":"                    instance, mountpoint, resize_to):"},{"line_number":504,"context_line":"        \"\"\"Replace the volume attached to the given `instance`."},{"line_number":505,"context_line":""},{"line_number":506,"context_line":"        :param dict old_connection_info:"},{"line_number":507,"context_line":"            The volume for this connection gets detached from the given"},{"line_number":508,"context_line":"            `instance`."}],"source_content_type":"text/x-python","patch_set":4,"id":"9a68dd71_fb4e49c0","line":505,"in_reply_to":"ba63e191_a14bcdaf","updated":"2016-01-21 05:45:18.000000000","message":"Done","commit_id":"36b75c370dfbd0c3125e5b9e4056ca2afb7733ce"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"d547c781073caf94f48b1a6f1daa7eade3d107a4","unresolved":false,"context_lines":[{"line_number":467,"context_line":"        \"\"\"Detach the disk attached to the instance.\"\"\""},{"line_number":468,"context_line":"        raise NotImplementedError()"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def swap_volume(self, context, old_connection_info, new_connection_info,"},{"line_number":471,"context_line":"                    old_encryption, new_encryption,"},{"line_number":472,"context_line":"                    instance, mountpoint, resize_to):"},{"line_number":473,"context_line":"        \"\"\"Replace the volume attached to the given `instance`."},{"line_number":474,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3afc51ec_356022a5","line":471,"range":{"start_line":470,"start_character":0,"end_line":471,"end_character":51},"updated":"2016-03-15 07:05:14.000000000","message":"hi lisa, this function is called by compute_manager, and this is the base class of driver, I see you only modify libvirt driver\u0027s defination, but what about other drivers like xen/hyper-v etc?\n\nwon\u0027t it be break other drivers?","commit_id":"62ed78719b6edbf36e52279f350158034d5318fe"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"ead9b17463e3a0d336849e9e1a67200d09e4d010","unresolved":false,"context_lines":[{"line_number":467,"context_line":"        \"\"\"Detach the disk attached to the instance.\"\"\""},{"line_number":468,"context_line":"        raise NotImplementedError()"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"    def swap_volume(self, context, old_connection_info, new_connection_info,"},{"line_number":471,"context_line":"                    old_encryption, new_encryption,"},{"line_number":472,"context_line":"                    instance, mountpoint, resize_to):"},{"line_number":473,"context_line":"        \"\"\"Replace the volume attached to the given `instance`."},{"line_number":474,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3afc51ec_a024f2c4","line":471,"range":{"start_line":470,"start_character":0,"end_line":471,"end_character":51},"in_reply_to":"3afc51ec_356022a5","updated":"2016-03-15 08:03:43.000000000","message":"Hi Eli, there is not implementations for this function in drivers like xen/hyper-v.","commit_id":"62ed78719b6edbf36e52279f350158034d5318fe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"09d4332933310585b085e6e2d19803c7568f0f91","unresolved":false,"context_lines":[{"line_number":488,"context_line":"        :param int resize_to:"},{"line_number":489,"context_line":"            If the new volume is larger than the old volume, it gets resized"},{"line_number":490,"context_line":"            to the given size (in Gigabyte) of `resize_to`."},{"line_number":491,"context_line":"        :param dict old_encryption:"},{"line_number":492,"context_line":"            The encryption info of old volume."},{"line_number":493,"context_line":"        :param dict new_encryption:"},{"line_number":494,"context_line":"            The encryption info of new volume."},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"        :return: None"},{"line_number":497,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"9abb7d3a_dcd41aed","line":494,"range":{"start_line":491,"start_character":0,"end_line":494,"end_character":46},"updated":"2016-06-01 16:51:06.000000000","message":"These are in the wrong order, they come before the \u0027instance\u0027 parameter.","commit_id":"1bec009d6c0dc873707d77b928fc6563d59363d9"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"61778301d7c402cb0cbd507184c1f8264de64c09","unresolved":false,"context_lines":[{"line_number":488,"context_line":"        :param int resize_to:"},{"line_number":489,"context_line":"            If the new volume is larger than the old volume, it gets resized"},{"line_number":490,"context_line":"            to the given size (in Gigabyte) of `resize_to`."},{"line_number":491,"context_line":"        :param dict old_encryption:"},{"line_number":492,"context_line":"            The encryption info of old volume."},{"line_number":493,"context_line":"        :param dict new_encryption:"},{"line_number":494,"context_line":"            The encryption info of new volume."},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"        :return: None"},{"line_number":497,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"9abb7d3a_5afbf378","line":494,"range":{"start_line":491,"start_character":0,"end_line":494,"end_character":46},"in_reply_to":"9abb7d3a_dcd41aed","updated":"2016-06-02 03:52:08.000000000","message":"Done","commit_id":"1bec009d6c0dc873707d77b928fc6563d59363d9"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"09d4332933310585b085e6e2d19803c7568f0f91","unresolved":false,"context_lines":[{"line_number":1402,"context_line":"        finally:"},{"line_number":1403,"context_line":"            self._host.write_instance_config(xml)"},{"line_number":1404,"context_line":""},{"line_number":1405,"context_line":"    def swap_volume(self, context, old_connection_info, new_connection_info,"},{"line_number":1406,"context_line":"                    old_encryption, new_encryption, instance,"},{"line_number":1407,"context_line":"                    mountpoint, resize_to):"},{"line_number":1408,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9abb7d3a_dc7a5abc","line":1405,"range":{"start_line":1405,"start_character":26,"end_line":1405,"end_character":33},"updated":"2016-06-01 16:51:06.000000000","message":"This parameter isn\u0027t used as far as I can tell, so not sure why it was added.","commit_id":"1bec009d6c0dc873707d77b928fc6563d59363d9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0b9ca1b5a87368f763821b2832e8664c623c97df","unresolved":false,"context_lines":[{"line_number":1402,"context_line":"        finally:"},{"line_number":1403,"context_line":"            self._host.write_instance_config(xml)"},{"line_number":1404,"context_line":""},{"line_number":1405,"context_line":"    def swap_volume(self, context, old_connection_info, new_connection_info,"},{"line_number":1406,"context_line":"                    old_encryption, new_encryption, instance,"},{"line_number":1407,"context_line":"                    mountpoint, resize_to):"},{"line_number":1408,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9abb7d3a_5d6295d9","line":1405,"range":{"start_line":1405,"start_character":26,"end_line":1405,"end_character":33},"in_reply_to":"9abb7d3a_9a76fb03","updated":"2016-06-02 21:06:37.000000000","message":"Ah yes, I missed that.","commit_id":"1bec009d6c0dc873707d77b928fc6563d59363d9"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"61778301d7c402cb0cbd507184c1f8264de64c09","unresolved":false,"context_lines":[{"line_number":1402,"context_line":"        finally:"},{"line_number":1403,"context_line":"            self._host.write_instance_config(xml)"},{"line_number":1404,"context_line":""},{"line_number":1405,"context_line":"    def swap_volume(self, context, old_connection_info, new_connection_info,"},{"line_number":1406,"context_line":"                    old_encryption, new_encryption, instance,"},{"line_number":1407,"context_line":"                    mountpoint, resize_to):"},{"line_number":1408,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9abb7d3a_9a76fb03","line":1405,"range":{"start_line":1405,"start_character":26,"end_line":1405,"end_character":33},"in_reply_to":"9abb7d3a_dc7a5abc","updated":"2016-06-02 03:52:08.000000000","message":"The parameter context is used when attaching encryptors Line 1430. \n\nDuring attaching, it needs to get key. The context is used to check whether the authority can access the key. \n\nhttps://github.com/openstack/nova/blob/master/nova/volume/encryptors/luks.py#L101","commit_id":"1bec009d6c0dc873707d77b928fc6563d59363d9"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"cdab5dc1e3b7a56be9414a172d9888151109b6c0","unresolved":false,"context_lines":[{"line_number":1305,"context_line":"        new_volume_id \u003d new_connection_info.get(\u0027serial\u0027)"},{"line_number":1306,"context_line":"        # Attach encryptors if encrypted volume"},{"line_number":1307,"context_line":"        encryption \u003d encryptors.get_encryption_metadata("},{"line_number":1308,"context_line":"            context, self._volume_api, new_volume_id, old_connection_info)"},{"line_number":1309,"context_line":""},{"line_number":1310,"context_line":"        if encryption:"},{"line_number":1311,"context_line":"            encryptor \u003d self._get_volume_encryptor(new_connection_info,"}],"source_content_type":"text/x-python","patch_set":13,"id":"1aa78d24_807f0ddf","line":1308,"range":{"start_line":1308,"start_character":54,"end_line":1308,"end_character":73},"updated":"2016-07-12 07:45:52.000000000","message":"Not sure if here is old_connection_info.","commit_id":"6e835017d424d60d9a80d09bde50aaeefb10b583"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"b2038246dce970c3e264f8d6f02f4ab6117ee150","unresolved":false,"context_lines":[{"line_number":1305,"context_line":"        new_volume_id \u003d new_connection_info.get(\u0027serial\u0027)"},{"line_number":1306,"context_line":"        # Attach encryptors if encrypted volume"},{"line_number":1307,"context_line":"        encryption \u003d encryptors.get_encryption_metadata("},{"line_number":1308,"context_line":"            context, self._volume_api, new_volume_id, old_connection_info)"},{"line_number":1309,"context_line":""},{"line_number":1310,"context_line":"        if encryption:"},{"line_number":1311,"context_line":"            encryptor \u003d self._get_volume_encryptor(new_connection_info,"}],"source_content_type":"text/x-python","patch_set":13,"id":"1aa78d24_4ebc01dc","line":1308,"range":{"start_line":1308,"start_character":54,"end_line":1308,"end_character":73},"in_reply_to":"1aa78d24_807f0ddf","updated":"2016-07-13 01:54:13.000000000","message":"yes, this is wrong.","commit_id":"6e835017d424d60d9a80d09bde50aaeefb10b583"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"cdab5dc1e3b7a56be9414a172d9888151109b6c0","unresolved":false,"context_lines":[{"line_number":1302,"context_line":"            self._disconnect_volume(new_connection_info, disk_dev)"},{"line_number":1303,"context_line":"            raise NotImplementedError(_(\"Swap only supports host devices\"))"},{"line_number":1304,"context_line":""},{"line_number":1305,"context_line":"        new_volume_id \u003d new_connection_info.get(\u0027serial\u0027)"},{"line_number":1306,"context_line":"        # Attach encryptors if encrypted volume"},{"line_number":1307,"context_line":"        encryption \u003d encryptors.get_encryption_metadata("},{"line_number":1308,"context_line":"            context, self._volume_api, new_volume_id, old_connection_info)"},{"line_number":1309,"context_line":""},{"line_number":1310,"context_line":"        if encryption:"},{"line_number":1311,"context_line":"            encryptor \u003d self._get_volume_encryptor(new_connection_info,"},{"line_number":1312,"context_line":"                                                   encryption)"},{"line_number":1313,"context_line":"            encryptor.attach_volume(context, **encryption)"},{"line_number":1314,"context_line":""},{"line_number":1315,"context_line":"        # Save updates made in connection_info when connect_volume was called"}],"source_content_type":"text/x-python","patch_set":13,"id":"1aa78d24_00f79d8b","line":1312,"range":{"start_line":1305,"start_character":0,"end_line":1312,"end_character":62},"updated":"2016-07-12 07:45:52.000000000","message":"Can we create a new private function to deal with the logic(it\u0027s duplicated with line 1324)\n\nthe new function could be like this:\n\ndef _has_encryption(connection_info)\n# return encryptor or None\n  volume_id \u003d connection_info.get(\u0027serial\u0027)\n  encryption \u003d encryptors.get_encryption_metadata( )\n  ..\n  if encryption:\n     return self._get_volume_encryptor(...)\n  else\n     return None","commit_id":"6e835017d424d60d9a80d09bde50aaeefb10b583"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"b2038246dce970c3e264f8d6f02f4ab6117ee150","unresolved":false,"context_lines":[{"line_number":1302,"context_line":"            self._disconnect_volume(new_connection_info, disk_dev)"},{"line_number":1303,"context_line":"            raise NotImplementedError(_(\"Swap only supports host devices\"))"},{"line_number":1304,"context_line":""},{"line_number":1305,"context_line":"        new_volume_id \u003d new_connection_info.get(\u0027serial\u0027)"},{"line_number":1306,"context_line":"        # Attach encryptors if encrypted volume"},{"line_number":1307,"context_line":"        encryption \u003d encryptors.get_encryption_metadata("},{"line_number":1308,"context_line":"            context, self._volume_api, new_volume_id, old_connection_info)"},{"line_number":1309,"context_line":""},{"line_number":1310,"context_line":"        if encryption:"},{"line_number":1311,"context_line":"            encryptor \u003d self._get_volume_encryptor(new_connection_info,"},{"line_number":1312,"context_line":"                                                   encryption)"},{"line_number":1313,"context_line":"            encryptor.attach_volume(context, **encryption)"},{"line_number":1314,"context_line":""},{"line_number":1315,"context_line":"        # Save updates made in connection_info when connect_volume was called"}],"source_content_type":"text/x-python","patch_set":13,"id":"1aa78d24_aee4850c","line":1312,"range":{"start_line":1305,"start_character":0,"end_line":1312,"end_character":62},"in_reply_to":"1aa78d24_00f79d8b","updated":"2016-07-13 01:54:13.000000000","message":"I created new function.","commit_id":"6e835017d424d60d9a80d09bde50aaeefb10b583"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"f1937649e7b2c53bddc6a0b22fc127cbd5756473","unresolved":false,"context_lines":[{"line_number":1282,"context_line":"        finally:"},{"line_number":1283,"context_line":"            self._host.write_instance_config(xml)"},{"line_number":1284,"context_line":""},{"line_number":1285,"context_line":"    def _get_encryptor(self, context, connection_info):"},{"line_number":1286,"context_line":"        volume_id \u003d connection_info.get(\u0027serial\u0027)"},{"line_number":1287,"context_line":"        encryption \u003d encryptors.get_encryption_metadata("},{"line_number":1288,"context_line":"            context, self._volume_api, volume_id, connection_info)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1aa78d24_39bc5ddb","line":1285,"range":{"start_line":1285,"start_character":4,"end_line":1285,"end_character":55},"updated":"2016-07-13 02:16:17.000000000","message":"hi Lisa,\n\nHow about we do attach/detach inside this function by passing a \u0027action\u0027 to _get_encryptor(if that we need to rename this function, maybe _handle_encryption_volume).\n\nThen we can call it like this:\n\nself._handle_encryption_volume(context, connection_info \u0027attach\u0027/\u0027detach\u0027), this will avoid return a tuple.","commit_id":"ae016a9e89650a57d108fdd2b32ba33a258af73e"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"36391208cf02ea52ead8cd10d0a8bc6c1221bb0e","unresolved":false,"context_lines":[{"line_number":1282,"context_line":"        finally:"},{"line_number":1283,"context_line":"            self._host.write_instance_config(xml)"},{"line_number":1284,"context_line":""},{"line_number":1285,"context_line":"    def _get_encryptor(self, context, connection_info):"},{"line_number":1286,"context_line":"        volume_id \u003d connection_info.get(\u0027serial\u0027)"},{"line_number":1287,"context_line":"        encryption \u003d encryptors.get_encryption_metadata("},{"line_number":1288,"context_line":"            context, self._volume_api, volume_id, connection_info)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1aa78d24_0eb83410","line":1285,"range":{"start_line":1285,"start_character":4,"end_line":1285,"end_character":55},"in_reply_to":"1aa78d24_39bc5ddb","updated":"2016-07-13 05:05:14.000000000","message":"In fact, I considered the way. But I think it is not clear. Why do you think it is bad to return tuple?","commit_id":"ae016a9e89650a57d108fdd2b32ba33a258af73e"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"f0d7b36f239bf3c278cdb06394b1e6b5efa196e7","unresolved":false,"context_lines":[{"line_number":1290,"context_line":"        if encryption:"},{"line_number":1291,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":1292,"context_line":"                                                   encryption)"},{"line_number":1293,"context_line":"        return (encryption, encryptor)"},{"line_number":1294,"context_line":""},{"line_number":1295,"context_line":"    def swap_volume(self, context, old_connection_info, new_connection_info,"},{"line_number":1296,"context_line":"                    instance, mountpoint, resize_to):"}],"source_content_type":"text/x-python","patch_set":16,"id":"dada55a8_80d1ff59","line":1293,"updated":"2016-07-19 18:36:28.000000000","message":"I personally don\u0027t like returning a tuple to control the subsequent control flow at invocation sites. Eli\u0027s suggestion seems like an improvement in clarity.","commit_id":"495a45283544d94fa1fc451307be9d5d78e91b81"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"bc120a1c6c471b4b79e6604078f722641fa4c9dc","unresolved":false,"context_lines":[{"line_number":1290,"context_line":"        if encryption:"},{"line_number":1291,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":1292,"context_line":"                                                   encryption)"},{"line_number":1293,"context_line":"        return (encryption, encryptor)"},{"line_number":1294,"context_line":""},{"line_number":1295,"context_line":"    def swap_volume(self, context, old_connection_info, new_connection_info,"},{"line_number":1296,"context_line":"                    instance, mountpoint, resize_to):"}],"source_content_type":"text/x-python","patch_set":16,"id":"dada55a8_218ba669","line":1293,"in_reply_to":"dada55a8_80d1ff59","updated":"2016-07-21 06:11:39.000000000","message":"Both you and Eli don\u0027t like the tuple. I update it.","commit_id":"495a45283544d94fa1fc451307be9d5d78e91b81"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"0e6471603e9a62e63158553da04f64f11019a870","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"        # Detach old volume encryptor, and then the volume can be detached"},{"line_number":1336,"context_line":"        # successfully."},{"line_number":1337,"context_line":"        self._handle_encryptor(context, old_connection_info,"},{"line_number":1338,"context_line":"                               action\u003d\u0027detach\u0027)"},{"line_number":1339,"context_line":"        self._disconnect_volume(old_connection_info, disk_dev)"},{"line_number":1340,"context_line":""},{"line_number":1341,"context_line":"    def _get_existing_domain_xml(self, instance, network_info,"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_4b43286c","line":1338,"updated":"2016-07-21 20:39:13.000000000","message":"This statement should probably be in a try...except block in case _handle_encryptor raises an error.","commit_id":"7387bed43516b3da9cd9d5acae97d0b5b0c26a8e"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"606c6cd28437515da9e54e9f20d21b0e6733df6b","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"        # Detach old volume encryptor, and then the volume can be detached"},{"line_number":1336,"context_line":"        # successfully."},{"line_number":1337,"context_line":"        self._handle_encryptor(context, old_connection_info,"},{"line_number":1338,"context_line":"                               action\u003d\u0027detach\u0027)"},{"line_number":1339,"context_line":"        self._disconnect_volume(old_connection_info, disk_dev)"},{"line_number":1340,"context_line":""},{"line_number":1341,"context_line":"    def _get_existing_domain_xml(self, instance, network_info,"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_8c723148","line":1338,"in_reply_to":"dada55a8_29406d35","updated":"2016-07-22 14:05:24.000000000","message":"Shouldn\u0027t _disconnect_volume be called even if the encryptor raises an exception? In that case, the volume has been disconnected so there\u0027s no chance that an unencrypted handle to the volume persists.","commit_id":"7387bed43516b3da9cd9d5acae97d0b5b0c26a8e"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"56b49aaf6d8df10e5d5fe299894d2f909e109b85","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"        # Detach old volume encryptor, and then the volume can be detached"},{"line_number":1336,"context_line":"        # successfully."},{"line_number":1337,"context_line":"        self._handle_encryptor(context, old_connection_info,"},{"line_number":1338,"context_line":"                               action\u003d\u0027detach\u0027)"},{"line_number":1339,"context_line":"        self._disconnect_volume(old_connection_info, disk_dev)"},{"line_number":1340,"context_line":""},{"line_number":1341,"context_line":"    def _get_existing_domain_xml(self, instance, network_info,"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_29406d35","line":1338,"in_reply_to":"dada55a8_4b43286c","updated":"2016-07-22 06:30:53.000000000","message":"I considered try...except here, but I didn\u0027t find out any things to do. As at this moment, the new volume is used. If exception happens in handler_encryptor, let the exception elevated is enough.","commit_id":"7387bed43516b3da9cd9d5acae97d0b5b0c26a8e"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"a050427b0501cb256409e0b7b9d10eb7665ea439","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"        # Detach old volume encryptor, and then the volume can be detached"},{"line_number":1336,"context_line":"        # successfully."},{"line_number":1337,"context_line":"        self._handle_encryptor(context, old_connection_info,"},{"line_number":1338,"context_line":"                               action\u003d\u0027detach\u0027)"},{"line_number":1339,"context_line":"        self._disconnect_volume(old_connection_info, disk_dev)"},{"line_number":1340,"context_line":""},{"line_number":1341,"context_line":"    def _get_existing_domain_xml(self, instance, network_info,"}],"source_content_type":"text/x-python","patch_set":18,"id":"dada55a8_fad2505a","line":1338,"in_reply_to":"dada55a8_8c723148","updated":"2016-07-25 00:55:19.000000000","message":"In fact, the call to _disconnect_volume will fail. As at this moment, device_path in connection_info refer to the decrypted device.","commit_id":"7387bed43516b3da9cd9d5acae97d0b5b0c26a8e"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9ea7f6af460cc0fdcd609e057a86dfb444df5d66","unresolved":false,"context_lines":[{"line_number":1304,"context_line":"        driver_bdm.save()"},{"line_number":1305,"context_line":""},{"line_number":1306,"context_line":"        self._swap_volume(guest, disk_dev, conf.source_path, resize_to)"},{"line_number":1307,"context_line":"        self._disconnect_volume(old_connection_info, disk_dev)"},{"line_number":1308,"context_line":""},{"line_number":1309,"context_line":"    def _get_existing_domain_xml(self, instance, network_info,"},{"line_number":1310,"context_line":"                                 block_device_info\u003dNone):"}],"source_content_type":"text/x-python","patch_set":20,"id":"9ad45d7e_699c970f","side":"PARENT","line":1307,"updated":"2016-08-10 14:26:12.000000000","message":"I don\u0027t follow your last comment on patch set 18:\n\n\u003e In fact, the call to _disconnect_volume will fail. As at this moment, device_path in connection_info refer to the decrypted device.\n\nIf _disconnect_volume will fail, why bother calling it at all?\n\nRegardless, I still think that _disconnect_volume should be in a finally block because the volume should be disconnected even if detaching the encryptor fails.","commit_id":"07d141ef0d38a6e1d6c00bdf0162b38dcbc3711d"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"70a26cb9c4fa2fa3469b767b247c1d02b3c2f462","unresolved":false,"context_lines":[{"line_number":1304,"context_line":"        driver_bdm.save()"},{"line_number":1305,"context_line":""},{"line_number":1306,"context_line":"        self._swap_volume(guest, disk_dev, conf.source_path, resize_to)"},{"line_number":1307,"context_line":"        self._disconnect_volume(old_connection_info, disk_dev)"},{"line_number":1308,"context_line":""},{"line_number":1309,"context_line":"    def _get_existing_domain_xml(self, instance, network_info,"},{"line_number":1310,"context_line":"                                 block_device_info\u003dNone):"}],"source_content_type":"text/x-python","patch_set":20,"id":"3ac371cc_d9519510","side":"PARENT","line":1307,"in_reply_to":"9ad45d7e_699c970f","updated":"2016-08-15 04:56:01.000000000","message":"Sorry I didn\u0027t describe it clearly.\n\nIf detaching encryptor of old volume fails, to disconnect old volume also will fail. \n\nAs the raw(iscsi etc) device is used by dmcrypt if encryptor detach fails, disconnect_volume fails. Also you may look into the bug\u0027s description: the error will be like https://bugs.launchpad.net/nova/+bug/1522233","commit_id":"07d141ef0d38a6e1d6c00bdf0162b38dcbc3711d"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9ea7f6af460cc0fdcd609e057a86dfb444df5d66","unresolved":false,"context_lines":[{"line_number":1285,"context_line":"            if action \u003d\u003d \u0027attach\u0027:"},{"line_number":1286,"context_line":"                encryptor.attach_volume(context, **encryption)"},{"line_number":1287,"context_line":"            elif action \u003d\u003d \u0027detach\u0027:"},{"line_number":1288,"context_line":"                encryptor.detach_volume(**encryption)"},{"line_number":1289,"context_line":""},{"line_number":1290,"context_line":"    def swap_volume(self, context, old_connection_info, new_connection_info,"},{"line_number":1291,"context_line":"                    instance, mountpoint, resize_to):"}],"source_content_type":"text/x-python","patch_set":20,"id":"9ad45d7e_892d8b00","line":1288,"updated":"2016-08-10 14:26:12.000000000","message":"I\u0027m personally a fan of raising an exception if the action doesn\u0027t match an excepted value. For example,\n\n  else:\n     raise NotImplemented(\u0027%s is not a supported action\u0027 % action)\n\nThat makes debugging easier if someone every specifies an invalid action.","commit_id":"9d9d3b085b26d15b0ce6f0a93ba990eb6561641f"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"70a26cb9c4fa2fa3469b767b247c1d02b3c2f462","unresolved":false,"context_lines":[{"line_number":1285,"context_line":"            if action \u003d\u003d \u0027attach\u0027:"},{"line_number":1286,"context_line":"                encryptor.attach_volume(context, **encryption)"},{"line_number":1287,"context_line":"            elif action \u003d\u003d \u0027detach\u0027:"},{"line_number":1288,"context_line":"                encryptor.detach_volume(**encryption)"},{"line_number":1289,"context_line":""},{"line_number":1290,"context_line":"    def swap_volume(self, context, old_connection_info, new_connection_info,"},{"line_number":1291,"context_line":"                    instance, mountpoint, resize_to):"}],"source_content_type":"text/x-python","patch_set":20,"id":"3ac371cc_9006d11e","line":1288,"in_reply_to":"9ad45d7e_892d8b00","updated":"2016-08-15 04:56:01.000000000","message":"Done","commit_id":"9d9d3b085b26d15b0ce6f0a93ba990eb6561641f"}]}