)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6211862a051de8befc8a4dc61ba517aca9de462a","unresolved":false,"context_lines":[{"line_number":14,"context_line":"detaches which hit this same race condition."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I983f80822a5c210929f33e1aa348a0fef91e890b"},{"line_number":17,"context_line":"Closes-Bug: #1374508"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7a740942_cd54b322","line":17,"updated":"2015-12-07 09:24:20.000000000","message":"I believe this bug would also be fixed with this?\n\nhttps://bugs.launchpad.net/nova/+bug/1436561","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":14,"context_line":"detaches which hit this same race condition."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I983f80822a5c210929f33e1aa348a0fef91e890b"},{"line_number":17,"context_line":"Closes-Bug: #1374508"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7a740942_024d81c3","line":17,"in_reply_to":"7a740942_cd54b322","updated":"2015-12-08 01:31:21.000000000","message":"I\u0027m not sure if that bug will get fixed as well as part of that - I was only able to confirm the reboot bug is fixed.  I tried reproducing the other one locally but wasn\u0027t able to (seems like that happens when there\u0027s high volume or network latency involved).\n\nHopefully the retry will do the trick, but if there\u0027s some deeper issue where the retry doesn\u0027t fix the issue, I think the detach will now fail instead of silently passing and leaving stray connections because now we verify detach removed device from config.\n\nWanted to bring this up since this change will likely change the behavior of that bug (either fix it or now detach would fail if device can\u0027t be removed from the XML after retry), but I\u0027m not able to recreate to see which.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"}],"nova/exception.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6211862a051de8befc8a4dc61ba517aca9de462a","unresolved":false,"context_lines":[{"line_number":620,"context_line":""},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"class DeviceDetachFailed(NovaException):"},{"line_number":623,"context_line":"    msg_fmt \u003d _(\"Device detach failed: %(reason)s)\")"},{"line_number":624,"context_line":""},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"class DeviceNotFound(NovaException):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_7f03e60d","line":623,"updated":"2015-12-07 09:24:20.000000000","message":"Same comment as Matt from below. Would be useful to have the device in the message...","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":620,"context_line":""},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"class DeviceDetachFailed(NovaException):"},{"line_number":623,"context_line":"    msg_fmt \u003d _(\"Device detach failed: %(reason)s)\")"},{"line_number":624,"context_line":""},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"class DeviceNotFound(NovaException):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_7d80aaf7","line":623,"in_reply_to":"7a740942_7f03e60d","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":623,"context_line":"    msg_fmt \u003d _(\"Device detach failed: %(reason)s)\")"},{"line_number":624,"context_line":""},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"class DeviceNotFound(NovaException):"},{"line_number":627,"context_line":"    msg_fmt \u003d _(\"Device not found.\")"},{"line_number":628,"context_line":""},{"line_number":629,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_df86ab64","line":626,"updated":"2015-12-06 15:41:23.000000000","message":"This should probably extend the NotFound type, right?","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":623,"context_line":"    msg_fmt \u003d _(\"Device detach failed: %(reason)s)\")"},{"line_number":624,"context_line":""},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"class DeviceNotFound(NovaException):"},{"line_number":627,"context_line":"    msg_fmt \u003d _(\"Device not found.\")"},{"line_number":628,"context_line":""},{"line_number":629,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_9d855608","line":626,"in_reply_to":"7a740942_df86ab64","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":624,"context_line":""},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"class DeviceNotFound(NovaException):"},{"line_number":627,"context_line":"    msg_fmt \u003d _(\"Device not found.\")"},{"line_number":628,"context_line":""},{"line_number":629,"context_line":""},{"line_number":630,"context_line":"class SnapshotNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_9f802348","line":627,"updated":"2015-12-06 15:41:23.000000000","message":"Would be good to have the device in the error message (name, id, path, whatever).","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":624,"context_line":""},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"class DeviceNotFound(NovaException):"},{"line_number":627,"context_line":"    msg_fmt \u003d _(\"Device not found.\")"},{"line_number":628,"context_line":""},{"line_number":629,"context_line":""},{"line_number":630,"context_line":"class SnapshotNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_3d7f8216","line":627,"in_reply_to":"7a740942_9f802348","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6eec0c93095b415b8ac455d5b9a87d98dc52f4d9","unresolved":false,"context_lines":[{"line_number":5134,"context_line":"            for state in (power_state.RUNNING, power_state.PAUSED):"},{"line_number":5135,"context_line":"                mock_dom.info.return_value \u003d [state, 512, 512, 2, 1234, 5678]"},{"line_number":5136,"context_line":"                mock_get_domain.return_value \u003d mock_dom"},{"line_number":5137,"context_line":""},{"line_number":5138,"context_line":"                drvr.detach_volume(connection_info, instance, \u0027/dev/vdc\u0027)"},{"line_number":5139,"context_line":""},{"line_number":5140,"context_line":"                mock_get_domain.assert_called_with(instance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_ec16c454","side":"PARENT","line":5137,"updated":"2015-10-05 19:26:53.000000000","message":"remove this unrelated change","commit_id":"8eef1666e01c60719495b665b676f4aedb9a260a"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"4f530f7b18e27801a1ce63d4b4555df431609e0c","unresolved":false,"context_lines":[{"line_number":5134,"context_line":"            for state in (power_state.RUNNING, power_state.PAUSED):"},{"line_number":5135,"context_line":"                mock_dom.info.return_value \u003d [state, 512, 512, 2, 1234, 5678]"},{"line_number":5136,"context_line":"                mock_get_domain.return_value \u003d mock_dom"},{"line_number":5137,"context_line":""},{"line_number":5138,"context_line":"                drvr.detach_volume(connection_info, instance, \u0027/dev/vdc\u0027)"},{"line_number":5139,"context_line":""},{"line_number":5140,"context_line":"                mock_get_domain.assert_called_with(instance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_627e3bb0","side":"PARENT","line":5137,"in_reply_to":"7a2fa921_ec16c454","updated":"2015-10-05 22:14:19.000000000","message":"Done","commit_id":"8eef1666e01c60719495b665b676f4aedb9a260a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":5170,"context_line":"        self.assertRaises(exception.DiskNotFound, drvr.detach_volume,"},{"line_number":5171,"context_line":"                          connection_info, instance, \u0027/dev/vdc\u0027)"},{"line_number":5172,"context_line":""},{"line_number":5173,"context_line":"        mock_get_domain.assert_called_with(instance)"},{"line_number":5174,"context_line":""},{"line_number":5175,"context_line":"    def test_multi_nic(self):"},{"line_number":5176,"context_line":"        network_info \u003d _fake_network_info(self.stubs, 2)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_9f390344","line":5173,"updated":"2015-12-06 15:41:23.000000000","message":"nit: if this is only called once, then assert_called_once_with would be better.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":5170,"context_line":"        self.assertRaises(exception.DiskNotFound, drvr.detach_volume,"},{"line_number":5171,"context_line":"                          connection_info, instance, \u0027/dev/vdc\u0027)"},{"line_number":5172,"context_line":""},{"line_number":5173,"context_line":"        mock_get_domain.assert_called_with(instance)"},{"line_number":5174,"context_line":""},{"line_number":5175,"context_line":"    def test_multi_nic(self):"},{"line_number":5176,"context_line":"        network_info \u003d _fake_network_info(self.stubs, 2)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_dd967e3e","line":5173,"in_reply_to":"7a740942_9f390344","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"}],"nova/tests/unit/virt/libvirt/test_guest.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":225,"context_line":"            return get_config(dev_path)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        retry_detach \u003d self.guest.detach_device_with_retry(get_config_wrapped,"},{"line_number":228,"context_line":"                                                            True, True,"},{"line_number":229,"context_line":"                                                            inc_sleep_time\u003d.01)"},{"line_number":230,"context_line":"        # Ensure we\u0027ve only done the initial detach call"},{"line_number":231,"context_line":"        self.domain.detachDeviceFlags.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_ffc5a73b","line":228,"updated":"2015-12-06 15:41:23.000000000","message":"I\u0027d prefer to see kwargs for the boolean args for clarity.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":225,"context_line":"            return get_config(dev_path)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        retry_detach \u003d self.guest.detach_device_with_retry(get_config_wrapped,"},{"line_number":228,"context_line":"                                                            True, True,"},{"line_number":229,"context_line":"                                                            inc_sleep_time\u003d.01)"},{"line_number":230,"context_line":"        # Ensure we\u0027ve only done the initial detach call"},{"line_number":231,"context_line":"        self.domain.detachDeviceFlags.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_5da36e9d","line":228,"in_reply_to":"7a740942_ffc5a73b","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        self.domain.detachDeviceFlags.reset_mock()"},{"line_number":239,"context_line":"        retry_detach()"},{"line_number":240,"context_line":"        # Should have two retries before we pretend device is detached"},{"line_number":241,"context_line":"        self.assertEqual(self.domain.detachDeviceFlags.call_count, 2)"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def test_detach_device_with_retry_detach_failure(self):"},{"line_number":244,"context_line":"        conf \u003d mock.Mock(spec\u003dvconfig.LibvirtConfigGuestDevice)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_ffdcc713","line":241,"updated":"2015-12-06 15:41:23.000000000","message":"nit: the args to assertEqual should be (expected, observed) since it impacts the output of the MismatchError if this fails.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        self.domain.detachDeviceFlags.reset_mock()"},{"line_number":239,"context_line":"        retry_detach()"},{"line_number":240,"context_line":"        # Should have two retries before we pretend device is detached"},{"line_number":241,"context_line":"        self.assertEqual(self.domain.detachDeviceFlags.call_count, 2)"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def test_detach_device_with_retry_detach_failure(self):"},{"line_number":244,"context_line":"        conf \u003d mock.Mock(spec\u003dvconfig.LibvirtConfigGuestDevice)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_3dfb027b","line":241,"in_reply_to":"7a740942_ffdcc713","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":247,"context_line":"        get_config \u003d mock.Mock(return_value\u003dconf)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        retry_detach \u003d self.guest.detach_device_with_retry("},{"line_number":250,"context_line":"            get_config, True, True, inc_sleep_time\u003d.01, max_retry_count\u003d3)"},{"line_number":251,"context_line":"        # Ensure we\u0027ve only done the initial detach call"},{"line_number":252,"context_line":"        self.domain.detachDeviceFlags.assert_called_once_with("},{"line_number":253,"context_line":"            \"\u003c/xml\u003e\", flags\u003d(fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG |"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_1fd8b31c","line":250,"updated":"2015-12-06 15:41:23.000000000","message":"Use kwargs for the boolean values for clarity.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":247,"context_line":"        get_config \u003d mock.Mock(return_value\u003dconf)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        retry_detach \u003d self.guest.detach_device_with_retry("},{"line_number":250,"context_line":"            get_config, True, True, inc_sleep_time\u003d.01, max_retry_count\u003d3)"},{"line_number":251,"context_line":"        # Ensure we\u0027ve only done the initial detach call"},{"line_number":252,"context_line":"        self.domain.detachDeviceFlags.assert_called_once_with("},{"line_number":253,"context_line":"            \"\u003c/xml\u003e\", flags\u003d(fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG |"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_5df84e7b","line":250,"in_reply_to":"7a740942_1fd8b31c","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":257,"context_line":"        self.domain.detachDeviceFlags.reset_mock()"},{"line_number":258,"context_line":"        # Should hit max # of retries"},{"line_number":259,"context_line":"        self.assertRaises(exception.DeviceDetachFailed, retry_detach)"},{"line_number":260,"context_line":"        # NOTE(rmcnair): currently RetryDecorator retries once more than"},{"line_number":261,"context_line":"        # max_retry_count, will circle back to fix this behavior"},{"line_number":262,"context_line":"        self.assertEqual(self.domain.detachDeviceFlags.call_count, 4)"},{"line_number":263,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_9f02636e","line":260,"updated":"2015-12-06 15:41:23.000000000","message":"You can\u0027t really change this in oslo.service without a backwards incompatible change, since it would change the behavior of the API and once released, this unit test would start failing and block all of nova unit test runs.\n\nSo if you plan on changing this in oslo.service, which I\u0027d argue is not actually necessary (probably), then you should be more lenient in this assertion, i.e. maybe assert that the call_count is between 3 and 4.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":257,"context_line":"        self.domain.detachDeviceFlags.reset_mock()"},{"line_number":258,"context_line":"        # Should hit max # of retries"},{"line_number":259,"context_line":"        self.assertRaises(exception.DeviceDetachFailed, retry_detach)"},{"line_number":260,"context_line":"        # NOTE(rmcnair): currently RetryDecorator retries once more than"},{"line_number":261,"context_line":"        # max_retry_count, will circle back to fix this behavior"},{"line_number":262,"context_line":"        self.assertEqual(self.domain.detachDeviceFlags.call_count, 4)"},{"line_number":263,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_1d4ac6ac","line":260,"in_reply_to":"7a740942_9f02636e","updated":"2015-12-08 01:31:21.000000000","message":"Good point - didn\u0027t think through the side-effects of making that change.  Agree that since it\u0027s very minor issue it\u0027s probably not worth fixing now.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        self.assertRaises(exception.DeviceDetachFailed, retry_detach)"},{"line_number":260,"context_line":"        # NOTE(rmcnair): currently RetryDecorator retries once more than"},{"line_number":261,"context_line":"        # max_retry_count, will circle back to fix this behavior"},{"line_number":262,"context_line":"        self.assertEqual(self.domain.detachDeviceFlags.call_count, 4)"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    def test_detach_device_with_retry_device_not_found(self):"},{"line_number":265,"context_line":"        get_config \u003d mock.Mock(return_value\u003dNone)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_df08eb93","line":262,"updated":"2015-12-06 15:41:23.000000000","message":"nit: swap the args","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        self.assertRaises(exception.DeviceDetachFailed, retry_detach)"},{"line_number":260,"context_line":"        # NOTE(rmcnair): currently RetryDecorator retries once more than"},{"line_number":261,"context_line":"        # max_retry_count, will circle back to fix this behavior"},{"line_number":262,"context_line":"        self.assertEqual(self.domain.detachDeviceFlags.call_count, 4)"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    def test_detach_device_with_retry_device_not_found(self):"},{"line_number":265,"context_line":"        get_config \u003d mock.Mock(return_value\u003dNone)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_bdc93228","line":262,"in_reply_to":"7a740942_df08eb93","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":265,"context_line":"        get_config \u003d mock.Mock(return_value\u003dNone)"},{"line_number":266,"context_line":"        self.assertRaises(exception.DeviceNotFound,"},{"line_number":267,"context_line":"                          self.guest.detach_device_with_retry, get_config,"},{"line_number":268,"context_line":"                          True, True)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def test_get_xml_desc(self):"},{"line_number":271,"context_line":"        self.guest.get_xml_desc()"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_3ffa4f76","line":268,"updated":"2015-12-06 15:41:23.000000000","message":"kwargs for booleans.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":265,"context_line":"        get_config \u003d mock.Mock(return_value\u003dNone)"},{"line_number":266,"context_line":"        self.assertRaises(exception.DeviceNotFound,"},{"line_number":267,"context_line":"                          self.guest.detach_device_with_retry, get_config,"},{"line_number":268,"context_line":"                          True, True)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def test_get_xml_desc(self):"},{"line_number":271,"context_line":"        self.guest.get_xml_desc()"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_ddce7e2d","line":268,"in_reply_to":"7a740942_3ffa4f76","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"}],"nova/tests/unit/virt/test_virt_drivers.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6eec0c93095b415b8ac455d5b9a87d98dc52f4d9","unresolved":false,"context_lines":[{"line_number":110,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":111,"context_line":"            \u0027nova.virt.libvirt.firewall.libvirt\u0027,"},{"line_number":112,"context_line":"            fakelibvirt))"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":115,"context_line":"            \u0027nova.virt.libvirt.driver.connector\u0027,"},{"line_number":116,"context_line":"            fake_os_brick_connector))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_2c36fc03","side":"PARENT","line":113,"updated":"2015-10-05 19:26:53.000000000","message":"remove this unrelated change","commit_id":"8eef1666e01c60719495b665b676f4aedb9a260a"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"4f530f7b18e27801a1ce63d4b4555df431609e0c","unresolved":false,"context_lines":[{"line_number":110,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":111,"context_line":"            \u0027nova.virt.libvirt.firewall.libvirt\u0027,"},{"line_number":112,"context_line":"            fakelibvirt))"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":115,"context_line":"            \u0027nova.virt.libvirt.driver.connector\u0027,"},{"line_number":116,"context_line":"            fake_os_brick_connector))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_0231870b","side":"PARENT","line":113,"in_reply_to":"7a2fa921_2c36fc03","updated":"2015-10-05 22:14:19.000000000","message":"Done","commit_id":"8eef1666e01c60719495b665b676f4aedb9a260a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6eec0c93095b415b8ac455d5b9a87d98dc52f4d9","unresolved":false,"context_lines":[{"line_number":473,"context_line":"            \"serial\": \"fake_serial\","},{"line_number":474,"context_line":"            \"data\": {}"},{"line_number":475,"context_line":"        }"},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"        self.assertIsNone("},{"line_number":478,"context_line":"            self.connection.attach_volume(None, connection_info, instance_ref,"},{"line_number":479,"context_line":"                                          \u0027/dev/sda\u0027))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_ac220cb8","line":476,"updated":"2015-10-05 19:26:53.000000000","message":"remove this unrelated change","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"4f530f7b18e27801a1ce63d4b4555df431609e0c","unresolved":false,"context_lines":[{"line_number":473,"context_line":"            \"serial\": \"fake_serial\","},{"line_number":474,"context_line":"            \"data\": {}"},{"line_number":475,"context_line":"        }"},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"        self.assertIsNone("},{"line_number":478,"context_line":"            self.connection.attach_volume(None, connection_info, instance_ref,"},{"line_number":479,"context_line":"                                          \u0027/dev/sda\u0027))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_622cfbab","line":476,"in_reply_to":"7a2fa921_ac220cb8","updated":"2015-10-05 22:14:19.000000000","message":"Done","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            # Still calling detach, but instead of returning function"},{"line_number":151,"context_line":"            # that actually checks if device is gone from XML, just continue"},{"line_number":152,"context_line":"            # because XML never gets updated in these tests"},{"line_number":153,"context_line":"            _self.detach_device(get_device_conf_func(), live, persistent)"},{"line_number":154,"context_line":"            return fake_wait"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        self.stubs.Set(nova.virt.libvirt.driver.LibvirtDriver,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_dfba2bc0","line":153,"updated":"2015-12-06 15:41:23.000000000","message":"I\u0027m not entirely sure how this is being used, but the live and persistent parameters here are in the wrong order.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6211862a051de8befc8a4dc61ba517aca9de462a","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            # Still calling detach, but instead of returning function"},{"line_number":151,"context_line":"            # that actually checks if device is gone from XML, just continue"},{"line_number":152,"context_line":"            # because XML never gets updated in these tests"},{"line_number":153,"context_line":"            _self.detach_device(get_device_conf_func(), live, persistent)"},{"line_number":154,"context_line":"            return fake_wait"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        self.stubs.Set(nova.virt.libvirt.driver.LibvirtDriver,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_ffca766d","line":153,"in_reply_to":"7a740942_dfba2bc0","updated":"2015-12-07 09:24:20.000000000","message":"Yup, Matt is correct. Making those kwarg parameters would solve your problem for you :)","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            # Still calling detach, but instead of returning function"},{"line_number":151,"context_line":"            # that actually checks if device is gone from XML, just continue"},{"line_number":152,"context_line":"            # because XML never gets updated in these tests"},{"line_number":153,"context_line":"            _self.detach_device(get_device_conf_func(), live, persistent)"},{"line_number":154,"context_line":"            return fake_wait"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        self.stubs.Set(nova.virt.libvirt.driver.LibvirtDriver,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_9deb1678","line":153,"in_reply_to":"7a740942_ffca766d","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"077dafe1be17d300e637835b41fc6cb20b1c0e81","unresolved":false,"context_lines":[{"line_number":1217,"context_line":"        conf \u003d guest.get_disk(disk_dev)"},{"line_number":1218,"context_line":"        # Stop looping once the disk no longer exists on the guest"},{"line_number":1219,"context_line":"        if conf is None:"},{"line_number":1220,"context_line":"            raise loopingcall.LoopingCallDone()"},{"line_number":1221,"context_line":"        else:"},{"line_number":1222,"context_line":"            # Device is already detached from persistent domain"},{"line_number":1223,"context_line":"            # and only transient domain needs update"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_924fe5bc","line":1220,"updated":"2015-09-26 19:39:17.000000000","message":"Do we have to worry about the case where this is never met?  Concerned we could loop forever.","commit_id":"6a1c51a548de8a22df5885872e9742e67c9d8b81"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"0ba7f16e44c57c60c2828204758eecc0671edaa4","unresolved":false,"context_lines":[{"line_number":1239,"context_line":"            # Eat exception if max # of tries hit for mapping removal"},{"line_number":1240,"context_line":"            pass"},{"line_number":1241,"context_line":""},{"line_number":1242,"context_line":""},{"line_number":1243,"context_line":"    def detach_volume(self, connection_info, instance, mountpoint,"},{"line_number":1244,"context_line":"                      encryption\u003dNone):"},{"line_number":1245,"context_line":"        disk_dev \u003d mountpoint.rpartition(\"/\")[2]"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_709d31bb","line":1242,"updated":"2015-09-29 01:40:14.000000000","message":"remove this line","commit_id":"758a9976f764d03cf22cd252c71be00421414953"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6eec0c93095b415b8ac455d5b9a87d98dc52f4d9","unresolved":false,"context_lines":[{"line_number":1206,"context_line":"                                      block_device_info\u003dblock_device_info)"},{"line_number":1207,"context_line":"        return xml"},{"line_number":1208,"context_line":""},{"line_number":1209,"context_line":"    @loopingcall.RetryDecorator(max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":1210,"context_line":"                                max_sleep_time\u003d30,"},{"line_number":1211,"context_line":"                                exceptions\u003d(exception.PciDeviceDetachFailed))"},{"line_number":1212,"context_line":"    def _retry_mapping_removal_from_guest_domain(self, guest, disk_dev, live):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_aca0cc91","line":1209,"updated":"2015-10-05 19:26:53.000000000","message":"I\u0027m not sure how crazy I am about using this since it\u0027s going to dump a stack trace warning each time it runs and handles the exception:\n\nhttps://github.com/openstack/oslo.service/blob/0.7.0/oslo_service/loopingcall.py#L235\n\nSeems like things would be cleaner if we just did our own retry backoff logic in the _wait_for_mapping_removal_from_guest_domain method and logged the warning when timing out.","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ecdd49910a5416e2738346dab6553694f3d01ad4","unresolved":false,"context_lines":[{"line_number":1206,"context_line":"                                      block_device_info\u003dblock_device_info)"},{"line_number":1207,"context_line":"        return xml"},{"line_number":1208,"context_line":""},{"line_number":1209,"context_line":"    @loopingcall.RetryDecorator(max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":1210,"context_line":"                                max_sleep_time\u003d30,"},{"line_number":1211,"context_line":"                                exceptions\u003d(exception.PciDeviceDetachFailed))"},{"line_number":1212,"context_line":"    def _retry_mapping_removal_from_guest_domain(self, guest, disk_dev, live):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_c7e641e8","line":1209,"in_reply_to":"7a2fa921_aca0cc91","updated":"2015-10-05 19:28:45.000000000","message":"I\u0027m going to push a change to the RetryDecorator in oslo.service so that it doesn\u0027t log a stacktrace on each loop (change that warning to debug) and do the trace in the LOG.error call - or maybe just debug only in the decorator and no warning/error.  The caller has the context on what it\u0027s trying to retry, I think it\u0027s kind of odd that the decorator utility is doing all of this warning/error logging because the messages it\u0027s giving are basically useless to an operator, they just log that the thing failed and give the function name.","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"4f530f7b18e27801a1ce63d4b4555df431609e0c","unresolved":false,"context_lines":[{"line_number":1206,"context_line":"                                      block_device_info\u003dblock_device_info)"},{"line_number":1207,"context_line":"        return xml"},{"line_number":1208,"context_line":""},{"line_number":1209,"context_line":"    @loopingcall.RetryDecorator(max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":1210,"context_line":"                                max_sleep_time\u003d30,"},{"line_number":1211,"context_line":"                                exceptions\u003d(exception.PciDeviceDetachFailed))"},{"line_number":1212,"context_line":"    def _retry_mapping_removal_from_guest_domain(self, guest, disk_dev, live):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_18881722","line":1209,"in_reply_to":"7a2fa921_c7e641e8","updated":"2015-10-05 22:14:19.000000000","message":"Good point - thanks for handling this with https://bugs.launchpad.net/oslo.service/+bug/1503017","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6eec0c93095b415b8ac455d5b9a87d98dc52f4d9","unresolved":false,"context_lines":[{"line_number":1208,"context_line":""},{"line_number":1209,"context_line":"    @loopingcall.RetryDecorator(max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":1210,"context_line":"                                max_sleep_time\u003d30,"},{"line_number":1211,"context_line":"                                exceptions\u003d(exception.PciDeviceDetachFailed))"},{"line_number":1212,"context_line":"    def _retry_mapping_removal_from_guest_domain(self, guest, disk_dev, live):"},{"line_number":1213,"context_line":"        \"\"\"Retry removing the device mapping if it is not yet removed"},{"line_number":1214,"context_line":"        from the guest domain XML. Needed because immediately after a"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_0c31c00b","line":1211,"updated":"2015-10-05 19:26:53.000000000","message":"You\u0027re waiting for a block device to be detached, not a PCI device, so this seems like the wrong exception to use, even if it\u0027s just a marker at this point.","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"4f530f7b18e27801a1ce63d4b4555df431609e0c","unresolved":false,"context_lines":[{"line_number":1208,"context_line":""},{"line_number":1209,"context_line":"    @loopingcall.RetryDecorator(max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":1210,"context_line":"                                max_sleep_time\u003d30,"},{"line_number":1211,"context_line":"                                exceptions\u003d(exception.PciDeviceDetachFailed))"},{"line_number":1212,"context_line":"    def _retry_mapping_removal_from_guest_domain(self, guest, disk_dev, live):"},{"line_number":1213,"context_line":"        \"\"\"Retry removing the device mapping if it is not yet removed"},{"line_number":1214,"context_line":"        from the guest domain XML. Needed because immediately after a"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_e4dc9848","line":1211,"in_reply_to":"7a2fa921_0c31c00b","updated":"2015-10-05 22:14:19.000000000","message":"Done","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6eec0c93095b415b8ac455d5b9a87d98dc52f4d9","unresolved":false,"context_lines":[{"line_number":1217,"context_line":"        case where the volume is not successfully detached from the"},{"line_number":1218,"context_line":"        guest on the first try."},{"line_number":1219,"context_line":"        \"\"\""},{"line_number":1220,"context_line":"        conf \u003d guest.get_disk(disk_dev)"},{"line_number":1221,"context_line":""},{"line_number":1222,"context_line":"        if conf is not None:"},{"line_number":1223,"context_line":"            # Device is already detached from persistent domain"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_6c7e34b0","line":1220,"updated":"2015-10-05 19:26:53.000000000","message":"I wonder why this is handled differently from _detach_pci_devices where it\u0027s explicitly re-parsing the domain xml after the detach (even though get_disk does that) and then looping over the conf.devices looking for any matches and then failing if one is still found.  Maybe it\u0027s just something that could be moved into the Guest object that needs to handle LibvirtConfigGuestHostdevPCI.","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"4f530f7b18e27801a1ce63d4b4555df431609e0c","unresolved":false,"context_lines":[{"line_number":1217,"context_line":"        case where the volume is not successfully detached from the"},{"line_number":1218,"context_line":"        guest on the first try."},{"line_number":1219,"context_line":"        \"\"\""},{"line_number":1220,"context_line":"        conf \u003d guest.get_disk(disk_dev)"},{"line_number":1221,"context_line":""},{"line_number":1222,"context_line":"        if conf is not None:"},{"line_number":1223,"context_line":"            # Device is already detached from persistent domain"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_c4c71c44","line":1220,"in_reply_to":"7a2fa921_6c7e34b0","updated":"2015-10-05 22:14:19.000000000","message":"I was doing this separately because for this race condition we don\u0027t want to fail if the detach didn\u0027t stick on the first try, we just want to wait and try it again.  My understanding is this differs from the intended behavior for _detach_pci_devices.  Unless you\u0027re thinking that perhaps this is the same root issue for PCI and block devices, and the wait/retry logic could be also be used in _detach_pci_devices instead of immediate failure?","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cd14093c23d53cdc23a904f3956fc5348e5e7081","unresolved":false,"context_lines":[{"line_number":1217,"context_line":"        case where the volume is not successfully detached from the"},{"line_number":1218,"context_line":"        guest on the first try."},{"line_number":1219,"context_line":"        \"\"\""},{"line_number":1220,"context_line":"        conf \u003d guest.get_disk(disk_dev)"},{"line_number":1221,"context_line":""},{"line_number":1222,"context_line":"        if conf is not None:"},{"line_number":1223,"context_line":"            # Device is already detached from persistent domain"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_7481eb49","line":1220,"in_reply_to":"7a2fa921_c4c71c44","updated":"2015-10-06 17:48:40.000000000","message":"No I have no idea of the history or thinking behind the PCI method.  Would probably be a better question for Robert Baoli (baoli in irc).","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6eec0c93095b415b8ac455d5b9a87d98dc52f4d9","unresolved":false,"context_lines":[{"line_number":1223,"context_line":"            # Device is already detached from persistent domain"},{"line_number":1224,"context_line":"            # and only transient domain needs update"},{"line_number":1225,"context_line":"            guest.detach_device(conf, persistent\u003dFalse, live\u003dlive)"},{"line_number":1226,"context_line":"            reason \u003d \"Device still present on guest transient domain\""},{"line_number":1227,"context_line":"            # Raise error since the disk still existed on the guest"},{"line_number":1228,"context_line":"            raise exception.PciDeviceDetachFailed(dev\u003ddisk_dev, reason\u003dreason)"},{"line_number":1229,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_2c659c44","line":1226,"updated":"2015-10-05 19:26:53.000000000","message":"This should be marked for translation if it\u0027s going to show up as a warning/error in the logs (which it would given the loopingcall.RetryDecorator code).","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"4f530f7b18e27801a1ce63d4b4555df431609e0c","unresolved":false,"context_lines":[{"line_number":1223,"context_line":"            # Device is already detached from persistent domain"},{"line_number":1224,"context_line":"            # and only transient domain needs update"},{"line_number":1225,"context_line":"            guest.detach_device(conf, persistent\u003dFalse, live\u003dlive)"},{"line_number":1226,"context_line":"            reason \u003d \"Device still present on guest transient domain\""},{"line_number":1227,"context_line":"            # Raise error since the disk still existed on the guest"},{"line_number":1228,"context_line":"            raise exception.PciDeviceDetachFailed(dev\u003ddisk_dev, reason\u003dreason)"},{"line_number":1229,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_24ddb014","line":1226,"in_reply_to":"7a2fa921_2c659c44","updated":"2015-10-05 22:14:19.000000000","message":"Done","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6eec0c93095b415b8ac455d5b9a87d98dc52f4d9","unresolved":false,"context_lines":[{"line_number":1236,"context_line":"            self._retry_mapping_removal_from_guest_domain(guest, disk_dev,"},{"line_number":1237,"context_line":"                                                          live)"},{"line_number":1238,"context_line":"        except exception.PciDeviceDetachFailed:"},{"line_number":1239,"context_line":"            # Eat exception if max # of tries hit for mapping removal"},{"line_number":1240,"context_line":"            pass"},{"line_number":1241,"context_line":""},{"line_number":1242,"context_line":"    def detach_volume(self, connection_info, instance, mountpoint,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_4cc81875","line":1239,"updated":"2015-10-05 19:26:53.000000000","message":"Why pass?  Why not let it fail?  Wouldn\u0027t you just be propagating the silent failure by ignoring it?","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"4f530f7b18e27801a1ce63d4b4555df431609e0c","unresolved":false,"context_lines":[{"line_number":1236,"context_line":"            self._retry_mapping_removal_from_guest_domain(guest, disk_dev,"},{"line_number":1237,"context_line":"                                                          live)"},{"line_number":1238,"context_line":"        except exception.PciDeviceDetachFailed:"},{"line_number":1239,"context_line":"            # Eat exception if max # of tries hit for mapping removal"},{"line_number":1240,"context_line":"            pass"},{"line_number":1241,"context_line":""},{"line_number":1242,"context_line":"    def detach_volume(self, connection_info, instance, mountpoint,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_e77b3a7a","line":1239,"in_reply_to":"7a2fa921_4cc81875","updated":"2015-10-05 22:14:19.000000000","message":"You\u0027re correct, these would leave the possibility of things not cleaned up properly, so I updated if we timeout","commit_id":"6854941d29084137beef7d6f7e1a5505b597f8b6"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9277771d06e5c77ddec681c13968f7ac79b20b44","unresolved":false,"context_lines":[{"line_number":1209,"context_line":"    @loopingcall.RetryDecorator(max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":1210,"context_line":"                                max_sleep_time\u003d30,"},{"line_number":1211,"context_line":"                                exceptions\u003dexception.VolumeBDMDetachError)"},{"line_number":1212,"context_line":"    def _wait_for_mapping_removal_from_guest_domain(self, guest, disk_dev,"},{"line_number":1213,"context_line":"                                                    live):"},{"line_number":1214,"context_line":"        \"\"\"Waits until disk_dev is removed from the guest domain, retrying"},{"line_number":1215,"context_line":"        the removal as necessary. Needed because immediately after a"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a2fa921_2b531fe3","line":1212,"updated":"2015-10-06 13:37:19.000000000","message":"This logic should be in Guest\u0027s object. Actually detach_device should just to return an async object to let opportunity to execute other processes when waiting.\n\nhttps://review.openstack.org/#/c/217680/2/nova/virt/libvirt/guest.py,cm","commit_id":"4f8d6e691bcca6c7a545d5ff7f788782965f7bc9"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"b68449ef5adbdc710ef71434e5295181271106c3","unresolved":false,"context_lines":[{"line_number":1209,"context_line":"    @loopingcall.RetryDecorator(max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":1210,"context_line":"                                max_sleep_time\u003d30,"},{"line_number":1211,"context_line":"                                exceptions\u003dexception.VolumeBDMDetachError)"},{"line_number":1212,"context_line":"    def _wait_for_mapping_removal_from_guest_domain(self, guest, disk_dev,"},{"line_number":1213,"context_line":"                                                    live):"},{"line_number":1214,"context_line":"        \"\"\"Waits until disk_dev is removed from the guest domain, retrying"},{"line_number":1215,"context_line":"        the removal as necessary. Needed because immediately after a"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a2fa921_9a73b415","line":1212,"in_reply_to":"7a2fa921_2b531fe3","updated":"2015-10-12 16:20:38.000000000","message":"I feel like the current behavior can already have the same asynchronous behavior you show in the patch - since this patch already separates the initial call to guest.detach_device (on line 1226) and waiting call (line 1244) we can do anything asynchronous between the two calls.\n\nI think it\u0027s a good point you bring up that we can do the encryptor detach before we start waiting/retrying, so would it be sufficient to move line 1244  below the encryptor.detach_volume call?\n\nThis would allow us to still have the asynchronous behavior on the first detach try.  Since we want the detach to succeed before leaving this detach_volume function, I don\u0027t see benefits we\u0027d get from making the retries async as well.","commit_id":"4f8d6e691bcca6c7a545d5ff7f788782965f7bc9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"db75832045cd539a184d382dbb395af6fc5bd4d5","unresolved":false,"context_lines":[{"line_number":1209,"context_line":"    @loopingcall.RetryDecorator(max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":1210,"context_line":"                                max_sleep_time\u003d30,"},{"line_number":1211,"context_line":"                                exceptions\u003dexception.VolumeBDMDetachError)"},{"line_number":1212,"context_line":"    def _wait_for_mapping_removal_from_guest_domain(self, guest, disk_dev,"},{"line_number":1213,"context_line":"                                                    live):"},{"line_number":1214,"context_line":"        \"\"\"Waits until disk_dev is removed from the guest domain, retrying"},{"line_number":1215,"context_line":"        the removal as necessary. Needed because immediately after a"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a2fa921_d9fcba40","line":1212,"in_reply_to":"7a2fa921_2b531fe3","updated":"2015-10-06 17:47:14.000000000","message":"So there are at least 4 different parties trying to fix this same thing now it seems:\n\n1. ryan\n\n2. sahid\n\n3. qiu yu\n\n4. mtanino\n\n--\n\nIt\u0027d be good if people could get together and talk about the best way to attack this problem.","commit_id":"4f8d6e691bcca6c7a545d5ff7f788782965f7bc9"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"ea37d8f3e1a533f2aecf04e6af0c821af9e1e3c5","unresolved":false,"context_lines":[{"line_number":1209,"context_line":"    @loopingcall.RetryDecorator(max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":1210,"context_line":"                                max_sleep_time\u003d30,"},{"line_number":1211,"context_line":"                                exceptions\u003dexception.VolumeBDMDetachError)"},{"line_number":1212,"context_line":"    def _wait_for_mapping_removal_from_guest_domain(self, guest, disk_dev,"},{"line_number":1213,"context_line":"                                                    live):"},{"line_number":1214,"context_line":"        \"\"\"Waits until disk_dev is removed from the guest domain, retrying"},{"line_number":1215,"context_line":"        the removal as necessary. Needed because immediately after a"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a2ca52d_927c0d22","line":1212,"in_reply_to":"7a2fa921_9a73b415","updated":"2015-10-13 13:12:46.000000000","message":"The point is probably to have one pattern to keep as example for other methods which may need it. In all cases we want this retry logic inside Guest\u0027s class. You can keep your RetryDecorator and to create an other method \"detach_device_and_wait_for_it\".\n\nActually I prefer my solution so I will probably update mine and remove dependency: https://review.openstack.org/#/c/217680/\n\nFeel free to update yours if you still think it\u0027s a better solution.","commit_id":"4f8d6e691bcca6c7a545d5ff7f788782965f7bc9"},{"author":{"_account_id":14358,"name":"Marian Horban","email":"m.horban@gmail.com","username":"mhorban"},"change_message_id":"03e1dddd9861a8047b947ffddbacf0d0cac23d44","unresolved":false,"context_lines":[{"line_number":1223,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(_get_guest_disk,"},{"line_number":1226,"context_line":"                                                             True, live)"},{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"            if encryption:"},{"line_number":1229,"context_line":"                # The volume must be detached from the VM before"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_7640a107","line":1226,"updated":"2015-11-27 16:17:57.000000000","message":"I investigated the same issue and retry logic helped me. \nBut with flag persistent\u003dTrue it can fail. \nBecause virDomainDetachDeviceFlags consists of detaching device from persistent config and live config.\nFirst call of virDomainDetachDeviceFlags will detach device from persistent config and try to detach device from live config.\nSecond call of virDomainDetachDeviceFlags could fail with error \"no target device\" because device is already deleted from persistent config. Detaching from persistent config is executed before detaching from live config in virDomainDetachDeviceFlags so in this case retry logic will not help.\nI think we should make two steps:\n1. detach device from persistent;\n2. retry detaching from live config only.","commit_id":"d9bd99598c5db2da1402ce4b9a1c5ee96087090d"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"5746949e10408ff8dcbfe216b00dd33c6e61c3c8","unresolved":false,"context_lines":[{"line_number":1223,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(_get_guest_disk,"},{"line_number":1226,"context_line":"                                                             True, live)"},{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"            if encryption:"},{"line_number":1229,"context_line":"                # The volume must be detached from the VM before"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_9637454c","line":1226,"in_reply_to":"9a8ffd7b_7640a107","updated":"2015-11-27 16:23:55.000000000","message":"The detach_device_with_retry method only uses the persistent flag for the first detach API call. For the looping calls it is always passing persistent\u003dFalse, so I think that\u0027 sok","commit_id":"d9bd99598c5db2da1402ce4b9a1c5ee96087090d"},{"author":{"_account_id":14358,"name":"Marian Horban","email":"m.horban@gmail.com","username":"mhorban"},"change_message_id":"670e568938f2e23526449a6ab1080f807963644a","unresolved":false,"context_lines":[{"line_number":1223,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":1224,"context_line":""},{"line_number":1225,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(_get_guest_disk,"},{"line_number":1226,"context_line":"                                                             True, live)"},{"line_number":1227,"context_line":""},{"line_number":1228,"context_line":"            if encryption:"},{"line_number":1229,"context_line":"                # The volume must be detached from the VM before"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_7875bac8","line":1226,"in_reply_to":"9a8ffd7b_9637454c","updated":"2015-11-30 12:51:16.000000000","message":"Oh, right you are!","commit_id":"d9bd99598c5db2da1402ce4b9a1c5ee96087090d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":1238,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":1239,"context_line":""},{"line_number":1240,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(_get_guest_disk,"},{"line_number":1241,"context_line":"                                                             True, live)"},{"line_number":1242,"context_line":""},{"line_number":1243,"context_line":"            if encryption:"},{"line_number":1244,"context_line":"                # The volume must be detached from the VM before"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_9fafc393","line":1241,"updated":"2015-12-06 15:41:23.000000000","message":"nit: I\u0027d still like to see persistent\u003dTrue rather than just a boolean value so the code is more self-explanatory.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":1238,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":1239,"context_line":""},{"line_number":1240,"context_line":"            wait_for_detach \u003d guest.detach_device_with_retry(_get_guest_disk,"},{"line_number":1241,"context_line":"                                                             True, live)"},{"line_number":1242,"context_line":""},{"line_number":1243,"context_line":"            if encryption:"},{"line_number":1244,"context_line":"                # The volume must be detached from the VM before"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_1d8f26cd","line":1241,"in_reply_to":"7a740942_9fafc393","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"}],"nova/virt/libvirt/guest.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"61be2c63a6c9021c0deb36552a9d7d74f59abb71","unresolved":false,"context_lines":[{"line_number":277,"context_line":"                devs.append(dev)"},{"line_number":278,"context_line":"        return devs"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def detach_device(self, conf, persistent\u003dFalse, live\u003dFalse):"},{"line_number":281,"context_line":"        \"\"\"Detaches device to the guest."},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"        :param conf: A LibvirtConfigObject of the device to detach"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_6ab79a86","side":"PARENT","line":280,"updated":"2015-10-15 08:09:02.000000000","message":"A first patch could be to change this and make detach_device to take a \u0027device\u0027 instead of a \u0027conf\u0027.","commit_id":"1d97b0e7308975cd4a912dda00df8726ba776fe7"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"4214299ccaef892cbc1cad6fcb60e66f8b742650","unresolved":false,"context_lines":[{"line_number":277,"context_line":"                devs.append(dev)"},{"line_number":278,"context_line":"        return devs"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def detach_device(self, conf, persistent\u003dFalse, live\u003dFalse):"},{"line_number":281,"context_line":"        \"\"\"Detaches device to the guest."},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"        :param conf: A LibvirtConfigObject of the device to detach"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_e77d0f1d","side":"PARENT","line":280,"in_reply_to":"5a2ca52d_6ab79a86","updated":"2015-10-15 14:24:03.000000000","message":"See comment on line 299 - I don\u0027t think that is the right approach here.","commit_id":"1d97b0e7308975cd4a912dda00df8726ba776fe7"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"61be2c63a6c9021c0deb36552a9d7d74f59abb71","unresolved":false,"context_lines":[{"line_number":278,"context_line":"                devs.append(dev)"},{"line_number":279,"context_line":"        return devs"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    def detach_device_async_retry(self, live, persistent, get_device_conf_func,"},{"line_number":282,"context_line":"                                  max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":283,"context_line":"                                  max_sleep_time\u003d30):"},{"line_number":284,"context_line":"        \"\"\"Detaches a device from the guest. After the initial detach request,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_aabb72e4","line":281,"updated":"2015-10-15 08:09:02.000000000","message":"You should to keep same order for arguments than detach_device(conf, persistent\u003dFalse, live\u003dFalse, max_try_count...)","commit_id":"a7c55cdde2438130a4bf8a60f951c9b5325d5bec"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"33f99d9c2e2c8bda8150228d60ac356526783d31","unresolved":false,"context_lines":[{"line_number":278,"context_line":"                devs.append(dev)"},{"line_number":279,"context_line":"        return devs"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    def detach_device_async_retry(self, live, persistent, get_device_conf_func,"},{"line_number":282,"context_line":"                                  max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":283,"context_line":"                                  max_sleep_time\u003d30):"},{"line_number":284,"context_line":"        \"\"\"Detaches a device from the guest. After the initial detach request,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_8860e7d0","line":281,"in_reply_to":"5a2ca52d_1d3c1ce1","updated":"2015-10-15 15:22:48.000000000","message":"Do you have an issue with the name or that the method is not behaving the in the asynchronous way you want it to?  The \"async\" part I had in mind was that after doing the initial call to detach, it returns the retry function which can be invoked by the caller at any later time to do the retry logic , which seems to be the along the lines of what you have in your patch here - https://review.openstack.org/#/c/217680/2/nova/virt/libvirt/guest.py,cm (with the difference this is retry vs. just wating)","commit_id":"a7c55cdde2438130a4bf8a60f951c9b5325d5bec"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"23306241391c972eea14fde82d97bf7e6a7146df","unresolved":false,"context_lines":[{"line_number":278,"context_line":"                devs.append(dev)"},{"line_number":279,"context_line":"        return devs"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    def detach_device_async_retry(self, live, persistent, get_device_conf_func,"},{"line_number":282,"context_line":"                                  max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":283,"context_line":"                                  max_sleep_time\u003d30):"},{"line_number":284,"context_line":"        \"\"\"Detaches a device from the guest. After the initial detach request,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a26ad4f_ac76dcd4","line":281,"in_reply_to":"5a2ca52d_8860e7d0","updated":"2015-11-03 08:44:38.000000000","message":"Name is not reflecting what this method is doing","commit_id":"a7c55cdde2438130a4bf8a60f951c9b5325d5bec"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"375a78ce8d93091c7d8260a9c5f76385bdb2bb93","unresolved":false,"context_lines":[{"line_number":278,"context_line":"                devs.append(dev)"},{"line_number":279,"context_line":"        return devs"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    def detach_device_async_retry(self, live, persistent, get_device_conf_func,"},{"line_number":282,"context_line":"                                  max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":283,"context_line":"                                  max_sleep_time\u003d30):"},{"line_number":284,"context_line":"        \"\"\"Detaches a device from the guest. After the initial detach request,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_1d3c1ce1","line":281,"in_reply_to":"5a2ca52d_aabb72e4","updated":"2015-10-15 15:05:04.000000000","message":"Just noticed that \u0027detach_device_ASYNC_retry\u0027 there is nothing async here. This method is blocking.","commit_id":"a7c55cdde2438130a4bf8a60f951c9b5325d5bec"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"61be2c63a6c9021c0deb36552a9d7d74f59abb71","unresolved":false,"context_lines":[{"line_number":296,"context_line":"                                     of the device to remove"},{"line_number":297,"context_line":"        \"\"\""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        conf \u003d get_device_conf_func()"},{"line_number":300,"context_line":"        if conf is None:"},{"line_number":301,"context_line":"            raise exception.DeviceNotFound"},{"line_number":302,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_caba6ec0","line":299,"updated":"2015-10-15 08:09:02.000000000","message":"Unfortunately problem comes here because detach_device takes a conf when it should take a device. - You should just to pass device and use self.get_disk()","commit_id":"a7c55cdde2438130a4bf8a60f951c9b5325d5bec"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"375a78ce8d93091c7d8260a9c5f76385bdb2bb93","unresolved":false,"context_lines":[{"line_number":296,"context_line":"                                     of the device to remove"},{"line_number":297,"context_line":"        \"\"\""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        conf \u003d get_device_conf_func()"},{"line_number":300,"context_line":"        if conf is None:"},{"line_number":301,"context_line":"            raise exception.DeviceNotFound"},{"line_number":302,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_2571bea8","line":299,"in_reply_to":"5a2ca52d_07928b57","updated":"2015-10-15 15:05:04.000000000","message":"In that case we may want specific methods like:\n\n def detach_disk_device(self, device, persistent\u003dFalse, live\u003dFalse):\n   return self.detach_device(self.get_disk(device), persistence, live)\n\n  def detach_vif_device(self, device...):\n    return  self.detach_device(...)","commit_id":"a7c55cdde2438130a4bf8a60f951c9b5325d5bec"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"33f99d9c2e2c8bda8150228d60ac356526783d31","unresolved":false,"context_lines":[{"line_number":296,"context_line":"                                     of the device to remove"},{"line_number":297,"context_line":"        \"\"\""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        conf \u003d get_device_conf_func()"},{"line_number":300,"context_line":"        if conf is None:"},{"line_number":301,"context_line":"            raise exception.DeviceNotFound"},{"line_number":302,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_a8b4c334","line":299,"in_reply_to":"5a2ca52d_2571bea8","updated":"2015-10-15 15:22:48.000000000","message":"Take a look at the detach_interface and detach_vif_device case - the config for those cases can not be easily grabbed by a function call present in guest.  So I don\u0027t know about trying to push all of that logic down into Guest.\n\nWhat is the benefit you see here from changing the flow of things to have these more granular methods in guest instead of using the existing flow and just making a small modification for the retry?","commit_id":"a7c55cdde2438130a4bf8a60f951c9b5325d5bec"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"23306241391c972eea14fde82d97bf7e6a7146df","unresolved":false,"context_lines":[{"line_number":296,"context_line":"                                     of the device to remove"},{"line_number":297,"context_line":"        \"\"\""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        conf \u003d get_device_conf_func()"},{"line_number":300,"context_line":"        if conf is None:"},{"line_number":301,"context_line":"            raise exception.DeviceNotFound"},{"line_number":302,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a26ad4f_6c7c44b5","line":299,"in_reply_to":"5a2ca52d_a8b4c334","updated":"2015-11-03 08:44:38.000000000","message":"Ok","commit_id":"a7c55cdde2438130a4bf8a60f951c9b5325d5bec"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"4214299ccaef892cbc1cad6fcb60e66f8b742650","unresolved":false,"context_lines":[{"line_number":296,"context_line":"                                     of the device to remove"},{"line_number":297,"context_line":"        \"\"\""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        conf \u003d get_device_conf_func()"},{"line_number":300,"context_line":"        if conf is None:"},{"line_number":301,"context_line":"            raise exception.DeviceNotFound"},{"line_number":302,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_07928b57","line":299,"in_reply_to":"5a2ca52d_caba6ec0","updated":"2015-10-15 14:24:03.000000000","message":"get_disk() is not the only possible path for grabbing the config of the device to remove.  detach_device is also called by _detach_pci_devices, detach_interface, and _detach_sriov_ports - which make different calls to grab the config to remove.\n\nWe\u0027re taking a function on line 299 so we can re-check if the device has been removed from the XML, and passing in any function lets this be generic so it can easily be leveraged by the other methods which don\u0027t use \"get_disks\" to grab device conf.","commit_id":"a7c55cdde2438130a4bf8a60f951c9b5325d5bec"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"5746949e10408ff8dcbfe216b00dd33c6e61c3c8","unresolved":false,"context_lines":[{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    def detach_device_with_retry(self, get_device_conf_func, persistent, live,"},{"line_number":282,"context_line":"                                  max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":283,"context_line":"                                  max_sleep_time\u003d30):"},{"line_number":284,"context_line":"        \"\"\"Detaches a device from the guest. After the initial detach request,"},{"line_number":285,"context_line":"        a function is returned which can be used to ensure the devices is"},{"line_number":286,"context_line":"        successfully removed from the guest domain (retrying the removal as"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_56c9dd57","line":283,"updated":"2015-11-27 16:23:55.000000000","message":"Super-nit-pick: indented 1 space too many - should line up with \u0027self\u0027","commit_id":"d9bd99598c5db2da1402ce4b9a1c5ee96087090d"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ab42f9db45f597a02f1ae5bb80a9dd2ad7b5dd9e","unresolved":false,"context_lines":[{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    def detach_device_with_retry(self, get_device_conf_func, persistent, live,"},{"line_number":282,"context_line":"                                  max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":283,"context_line":"                                  max_sleep_time\u003d30):"},{"line_number":284,"context_line":"        \"\"\"Detaches a device from the guest. After the initial detach request,"},{"line_number":285,"context_line":"        a function is returned which can be used to ensure the devices is"},{"line_number":286,"context_line":"        successfully removed from the guest domain (retrying the removal as"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_e75b3df5","line":283,"in_reply_to":"9a8ffd7b_56c9dd57","updated":"2015-11-30 16:04:05.000000000","message":"Done","commit_id":"d9bd99598c5db2da1402ce4b9a1c5ee96087090d"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"5746949e10408ff8dcbfe216b00dd33c6e61c3c8","unresolved":false,"context_lines":[{"line_number":293,"context_line":"        :param persistent: A bool to indicate whether the change is"},{"line_number":294,"context_line":"                           persistent or not"},{"line_number":295,"context_line":"        :param get_device_conf_func: function which returns the configuration"},{"line_number":296,"context_line":"                                     of the device to remove"},{"line_number":297,"context_line":"        \"\"\""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        conf \u003d get_device_conf_func()"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_d695cd32","line":296,"updated":"2015-11-27 16:23:55.000000000","message":"You\u0027ve missed docs for 3 parameters here.","commit_id":"d9bd99598c5db2da1402ce4b9a1c5ee96087090d"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ab42f9db45f597a02f1ae5bb80a9dd2ad7b5dd9e","unresolved":false,"context_lines":[{"line_number":293,"context_line":"        :param persistent: A bool to indicate whether the change is"},{"line_number":294,"context_line":"                           persistent or not"},{"line_number":295,"context_line":"        :param get_device_conf_func: function which returns the configuration"},{"line_number":296,"context_line":"                                     of the device to remove"},{"line_number":297,"context_line":"        \"\"\""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        conf \u003d get_device_conf_func()"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_8771117a","line":296,"in_reply_to":"9a8ffd7b_d695cd32","updated":"2015-11-30 16:04:05.000000000","message":"Done","commit_id":"d9bd99598c5db2da1402ce4b9a1c5ee96087090d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                                 max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":283,"context_line":"                                 max_sleep_time\u003d30):"},{"line_number":284,"context_line":"        \"\"\"Detaches a device from the guest. After the initial detach request,"},{"line_number":285,"context_line":"        a function is returned which can be used to ensure the devices is"},{"line_number":286,"context_line":"        successfully removed from the guest domain (retrying the removal as"},{"line_number":287,"context_line":"        necessary). Needed because immediately after a guest reboot"},{"line_number":288,"context_line":"        libvirtmod.virDomainDetachDeviceFlags() will silently fail to remove"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_3fc6cf9a","line":285,"updated":"2015-12-06 15:41:23.000000000","message":"nit: s/devices/device/","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                                 max_retry_count\u003d7, inc_sleep_time\u003d2,"},{"line_number":283,"context_line":"                                 max_sleep_time\u003d30):"},{"line_number":284,"context_line":"        \"\"\"Detaches a device from the guest. After the initial detach request,"},{"line_number":285,"context_line":"        a function is returned which can be used to ensure the devices is"},{"line_number":286,"context_line":"        successfully removed from the guest domain (retrying the removal as"},{"line_number":287,"context_line":"        necessary). Needed because immediately after a guest reboot"},{"line_number":288,"context_line":"        libvirtmod.virDomainDetachDeviceFlags() will silently fail to remove"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_dd9c3e11","line":285,"in_reply_to":"7a740942_3fc6cf9a","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":284,"context_line":"        \"\"\"Detaches a device from the guest. After the initial detach request,"},{"line_number":285,"context_line":"        a function is returned which can be used to ensure the devices is"},{"line_number":286,"context_line":"        successfully removed from the guest domain (retrying the removal as"},{"line_number":287,"context_line":"        necessary). Needed because immediately after a guest reboot"},{"line_number":288,"context_line":"        libvirtmod.virDomainDetachDeviceFlags() will silently fail to remove"},{"line_number":289,"context_line":"        the mapping."},{"line_number":290,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_ffbfe7f8","line":287,"updated":"2015-12-06 15:41:23.000000000","message":"I\u0027m not sure we need the \u0027Needed because...\u0027 in here, that belongs in the commit message or the code that\u0027s calling this.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":284,"context_line":"        \"\"\"Detaches a device from the guest. After the initial detach request,"},{"line_number":285,"context_line":"        a function is returned which can be used to ensure the devices is"},{"line_number":286,"context_line":"        successfully removed from the guest domain (retrying the removal as"},{"line_number":287,"context_line":"        necessary). Needed because immediately after a guest reboot"},{"line_number":288,"context_line":"        libvirtmod.virDomainDetachDeviceFlags() will silently fail to remove"},{"line_number":289,"context_line":"        the mapping."},{"line_number":290,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_7db42a81","line":287,"in_reply_to":"7a740942_ffbfe7f8","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        if conf is None:"},{"line_number":309,"context_line":"            raise exception.DeviceNotFound"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        self.detach_device(conf, persistent, live)"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"        @loopingcall.RetryDecorator(max_retry_count\u003dmax_retry_count,"},{"line_number":314,"context_line":"                                    inc_sleep_time\u003dinc_sleep_time,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_df3b0b9b","line":311,"updated":"2015-12-06 15:41:23.000000000","message":"Should you just pass persistent\u003dTrue here since you force persistent\u003dFalse below?","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        if conf is None:"},{"line_number":309,"context_line":"            raise exception.DeviceNotFound"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        self.detach_device(conf, persistent, live)"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"        @loopingcall.RetryDecorator(max_retry_count\u003dmax_retry_count,"},{"line_number":314,"context_line":"                                    inc_sleep_time\u003dinc_sleep_time,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_fd049a18","line":311,"in_reply_to":"7a740942_df3b0b9b","updated":"2015-12-08 01:31:21.000000000","message":"Although this method is currently only called with persistent\u003dTrue I wanted to keep this method generic so it can possibly be leveraged by places that use persistent\u003dFalse too.\n\nThe reason below we need to explicitly state persistent\u003dFalse is because we only want to try removing from persistent domain once.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d152de23e42efc948fb33ad547891649b08af92","unresolved":false,"context_lines":[{"line_number":321,"context_line":"                # and only transient domain needs update"},{"line_number":322,"context_line":"                self.detach_device(config, persistent\u003dFalse, live\u003dlive)"},{"line_number":323,"context_line":"                # Raise error since the device still existed on the guest"},{"line_number":324,"context_line":"                reason \u003d _(\"Unable to detach from guest transient domain.\")"},{"line_number":325,"context_line":"                raise exception.DeviceDetachFailed(reason\u003dreason)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        return _do_wait_and_retry_detach"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_7f59b7fd","line":324,"updated":"2015-12-06 15:41:23.000000000","message":"Would probably be good to have some device identifier (id/name/path) in here.","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"ee009077e938662782ffba65214da7c906ea2ea0","unresolved":false,"context_lines":[{"line_number":321,"context_line":"                # and only transient domain needs update"},{"line_number":322,"context_line":"                self.detach_device(config, persistent\u003dFalse, live\u003dlive)"},{"line_number":323,"context_line":"                # Raise error since the device still existed on the guest"},{"line_number":324,"context_line":"                reason \u003d _(\"Unable to detach from guest transient domain.\")"},{"line_number":325,"context_line":"                raise exception.DeviceDetachFailed(reason\u003dreason)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        return _do_wait_and_retry_detach"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a740942_b8014007","line":324,"in_reply_to":"7a740942_7f59b7fd","updated":"2015-12-08 01:31:21.000000000","message":"Done","commit_id":"db6c4ef3ec193517870f9efb176bb2afd6194a76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7679dccaa9063a03eacdf839cd5df61a7cb66989","unresolved":false,"context_lines":[{"line_number":321,"context_line":"                # and only transient domain needs update"},{"line_number":322,"context_line":"                self.detach_device(config, persistent\u003dFalse, live\u003dlive)"},{"line_number":323,"context_line":"                # Raise error since the device still existed on the guest"},{"line_number":324,"context_line":"                reason \u003d _(\"Unable to detach from guest transient domain.\")"},{"line_number":325,"context_line":"                raise exception.DeviceDetachFailed(device\u003ddevice,"},{"line_number":326,"context_line":"                                                   reason\u003dreason)"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a7e1126_c1d414ed","line":324,"updated":"2015-12-21 22:02:17.000000000","message":"So far we have 36 failures with this in the logs (in voting jobs):\n\nhttp://logstash.openstack.org/#dashboard/file/logstash.json?query\u003dmessage:%5C%22Unable%20to%20detach%20from%20guest%20transient%20domain%5C%22%20AND%20voting:%5C%221%5C%22%20AND%20tags:%5C%22screen-n-cpu.txt%5C%22\n\nIt\u0027s only manila jobs (which I think are probably slower) and it\u0027s only on OVH nodes (which are the slowest nodes we have).\n\nSo at least we have some idea of what\u0027s going on in those jobs. The thing I wonder is if the timeout should be configurable.","commit_id":"3a3fb3cfb2c41ad182545e47649ff12a4f3a743e"}]}