)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6655f9b5a678003a70f107ee627d162666830f86","unresolved":false,"context_lines":[{"line_number":17,"context_line":"This change introduces a simple call to the has_persistent_configuration"},{"line_number":18,"context_line":"to confirm the state of the domain before providing this to"},{"line_number":19,"context_line":"detach_device_with_retry to ensure the correct flags are used."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I95948721a0119f5f54dbe50d4455fd47d422164b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba2be162_d08efbce","line":20,"updated":"2017-03-03 17:13:25.000000000","message":"Do you plan on backporting this? If so, do you have a bug to track against?","commit_id":"3d60b439eea1ddf9f08b91be41583be68bb7773a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fc05d0282ec43e27b7d00e5cc35cc28ab3a0c797","unresolved":false,"context_lines":[{"line_number":19,"context_line":"within detach_device_with_retry to confirm the state of the domain"},{"line_number":20,"context_line":"before attempting to detach."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Closes-bug: #1669857"},{"line_number":23,"context_line":"Change-Id: I95948721a0119f5f54dbe50d4455fd47d422164b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"df140735_daf56acc","line":22,"updated":"2017-06-06 14:33:00.000000000","message":"We could also target this to bug 1696125 for detach_interface.","commit_id":"81ca30df61abd4b33f63354fc163682a19567364"}],"nova/tests/unit/virt/libvirt/test_guest.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"819b927856600beb800ea224e0877ddfdcf7665b","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        dev_path \u003d \"/dev/vdb\""},{"line_number":235,"context_line":"        self.domain.isPersistent.return_value \u003d False"},{"line_number":236,"context_line":"        retry_detach \u003d self.guest.detach_device_with_retry("},{"line_number":237,"context_line":"            get_config, dev_path, live\u003dFalse, inc_sleep_time\u003d.01)"},{"line_number":238,"context_line":"        self.domain.detachDeviceFlags.assert_called_once_with(\"\u003c/xml\u003e\","},{"line_number":239,"context_line":"                                                              flags\u003d0)"},{"line_number":240,"context_line":"        self.domain.detachDeviceFlags.reset_mock()"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_ad1e7c6d","line":237,"range":{"start_line":237,"start_character":34,"end_line":237,"end_character":44},"updated":"2017-03-10 23:12:07.000000000","message":"What is this test aiming to do? The name \"test_detach_device_with_retry_from_transient_domain\" implies a detach from a live domain but this is passing live\u003dFalse and flags end up as 0?","commit_id":"f7117b7fe1f31f835c2a774e8bf870a1a11d9d0c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"819b927856600beb800ea224e0877ddfdcf7665b","unresolved":false,"context_lines":[{"line_number":237,"context_line":"            get_config, dev_path, live\u003dFalse, inc_sleep_time\u003d.01)"},{"line_number":238,"context_line":"        self.domain.detachDeviceFlags.assert_called_once_with(\"\u003c/xml\u003e\","},{"line_number":239,"context_line":"                                                              flags\u003d0)"},{"line_number":240,"context_line":"        self.domain.detachDeviceFlags.reset_mock()"},{"line_number":241,"context_line":"        retry_detach()"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def test_detach_device_with_retry_detach_success(self):"},{"line_number":244,"context_line":"        conf \u003d mock.Mock(spec\u003dvconfig.LibvirtConfigGuestDevice)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_4d06e020","line":241,"range":{"start_line":240,"start_character":8,"end_line":241,"end_character":22},"updated":"2017-03-10 23:12:07.000000000","message":"Also not sure what the purpose of these statements are since you\u0027re not asserting anything afterward?","commit_id":"f7117b7fe1f31f835c2a774e8bf870a1a11d9d0c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d32c122203bd601271d2182b8e3ba33c76630567","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        dev_path \u003d \"/dev/vdb\""},{"line_number":236,"context_line":"        self.domain.isPersistent.return_value \u003d False"},{"line_number":237,"context_line":"        retry_detach \u003d self.guest.detach_device_with_retry("},{"line_number":238,"context_line":"            get_config, dev_path, live\u003dFalse, inc_sleep_time\u003d.01)"},{"line_number":239,"context_line":"        self.domain.detachDeviceFlags.assert_called_once_with(\"\u003c/xml\u003e\","},{"line_number":240,"context_line":"                                                              flags\u003d0)"},{"line_number":241,"context_line":"        self.domain.detachDeviceFlags.reset_mock()"}],"source_content_type":"text/x-python","patch_set":8,"id":"df140735_27e5590b","line":238,"range":{"start_line":238,"start_character":34,"end_line":238,"end_character":44},"updated":"2017-06-07 02:23:30.000000000","message":"What is this testing if both persistent and live are False? Wouldn\u0027t it just be a no-op? I thought we would want to test what happens if isPersistent is False and live is True, and assert that the VIR_DOMAIN_AFFECT_LIVE flag was passed in the call.\n\nThis is mostly based on the fact that the test is called \"test_detach_device_with_retry_from_transient_domain\" and it doesn\u0027t do that with live\u003dFalse. It\u0027s not persistent or transient.","commit_id":"d4fbb700588c81876f66eca5e5bb7a59cfe4b693"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bbb8cf9c5f9a6154a6c51a83299b538225814326","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        dev_path \u003d \"/dev/vdb\""},{"line_number":236,"context_line":"        self.domain.isPersistent.return_value \u003d False"},{"line_number":237,"context_line":"        retry_detach \u003d self.guest.detach_device_with_retry("},{"line_number":238,"context_line":"            get_config, dev_path, live\u003dFalse, inc_sleep_time\u003d.01)"},{"line_number":239,"context_line":"        self.domain.detachDeviceFlags.assert_called_once_with(\"\u003c/xml\u003e\","},{"line_number":240,"context_line":"                                                              flags\u003d0)"},{"line_number":241,"context_line":"        self.domain.detachDeviceFlags.reset_mock()"}],"source_content_type":"text/x-python","patch_set":8,"id":"df140735_a426f967","line":238,"range":{"start_line":238,"start_character":34,"end_line":238,"end_character":44},"in_reply_to":"df140735_27e5590b","updated":"2017-06-07 10:16:28.000000000","message":"I can change this to pass live\u003dTrue.","commit_id":"d4fbb700588c81876f66eca5e5bb7a59cfe4b693"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d32c122203bd601271d2182b8e3ba33c76630567","unresolved":false,"context_lines":[{"line_number":239,"context_line":"        self.domain.detachDeviceFlags.assert_called_once_with(\"\u003c/xml\u003e\","},{"line_number":240,"context_line":"                                                              flags\u003d0)"},{"line_number":241,"context_line":"        self.domain.detachDeviceFlags.reset_mock()"},{"line_number":242,"context_line":"        retry_detach()"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    def test_detach_device_with_retry_detach_success(self):"},{"line_number":245,"context_line":"        conf \u003d mock.Mock(spec\u003dvconfig.LibvirtConfigGuestDevice)"}],"source_content_type":"text/x-python","patch_set":8,"id":"df140735_87d66527","line":242,"updated":"2017-06-07 02:23:30.000000000","message":"Shouldn\u0027t there be an assertion of some kind after calling retry_detach()? Like the one at L252 to show that retry_detach() did something?","commit_id":"d4fbb700588c81876f66eca5e5bb7a59cfe4b693"}],"nova/tests/unit/virt/test_virt_drivers.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d32c122203bd601271d2182b8e3ba33c76630567","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                                          max_retry_count\u003d7,"},{"line_number":141,"context_line":"                                          inc_sleep_time\u003d2,"},{"line_number":142,"context_line":"                                          max_sleep_time\u003d30,"},{"line_number":143,"context_line":"                                          alternative_device_name\u003dNone):"},{"line_number":144,"context_line":"            # Still calling detach, but instead of returning function"},{"line_number":145,"context_line":"            # that actually checks if device is gone from XML, just continue"},{"line_number":146,"context_line":"            # because XML never gets updated in these tests"}],"source_content_type":"text/x-python","patch_set":8,"id":"df140735_a72cc94a","line":143,"updated":"2017-06-07 02:23:30.000000000","message":"This change looks unrelated?","commit_id":"d4fbb700588c81876f66eca5e5bb7a59cfe4b693"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bbb8cf9c5f9a6154a6c51a83299b538225814326","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                                          max_retry_count\u003d7,"},{"line_number":141,"context_line":"                                          inc_sleep_time\u003d2,"},{"line_number":142,"context_line":"                                          max_sleep_time\u003d30,"},{"line_number":143,"context_line":"                                          alternative_device_name\u003dNone):"},{"line_number":144,"context_line":"            # Still calling detach, but instead of returning function"},{"line_number":145,"context_line":"            # that actually checks if device is gone from XML, just continue"},{"line_number":146,"context_line":"            # because XML never gets updated in these tests"}],"source_content_type":"text/x-python","patch_set":8,"id":"df140735_04fec5ad","line":143,"in_reply_to":"df140735_a72cc94a","updated":"2017-06-07 10:16:28.000000000","message":"Maybe, I added this during a rebase. Apparently I missed this signature change in https://review.openstack.org/#/c/349014/ - it\u0027s odd that it doesn\u0027t cause any failures. Maybe we should just turn everything after the \"live\" arg into *args/**kwargs since we don\u0027t use them.","commit_id":"d4fbb700588c81876f66eca5e5bb7a59cfe4b693"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"ae98edaffc7f929d5fab71fc4bc1797b40dc6ccb","unresolved":false,"context_lines":[{"line_number":1330,"context_line":"            # from its encryptor. Otherwise, the encryptor may report that the"},{"line_number":1331,"context_line":"            # volume is still in use."},{"line_number":1332,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(guest.get_disk,"},{"line_number":1333,"context_line":"                                disk_dev, persistent\u003dpersistent, live\u003dlive)"},{"line_number":1334,"context_line":"            wait_for_detach()"},{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":"            if encryption:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba2be162_106a53ee","line":1333,"updated":"2017-03-03 17:17:50.000000000","message":"I wonder if the default behaviour of detach_device_with_retry() should just be \u0027do the thing that will work\u0027. In fact, if there\u0027s no explicit reason anywhere to set persistent explicitly, perhaps we could remove the argument? Just seems odd to query the guest for this value before passing it straight back.","commit_id":"3d60b439eea1ddf9f08b91be41583be68bb7773a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d5816140ae6cd3cb608e02b982802d30f512cf1d","unresolved":false,"context_lines":[{"line_number":1330,"context_line":"            # from its encryptor. Otherwise, the encryptor may report that the"},{"line_number":1331,"context_line":"            # volume is still in use."},{"line_number":1332,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(guest.get_disk,"},{"line_number":1333,"context_line":"                                disk_dev, persistent\u003dpersistent, live\u003dlive)"},{"line_number":1334,"context_line":"            wait_for_detach()"},{"line_number":1335,"context_line":""},{"line_number":1336,"context_line":"            if encryption:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba2be162_60a49ab9","line":1333,"in_reply_to":"ba2be162_106a53ee","updated":"2017-03-03 19:08:43.000000000","message":"Yeah I\u0027d be in favor of just removing the persistent arg from detach_device_with_retry.","commit_id":"3d60b439eea1ddf9f08b91be41583be68bb7773a"}]}
