)]}'
{"nova/tests/unit/virt/xenapi/test_vif.py":[{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"64fdf4f4c7d5bfaf89767fe0de95283230949fd3","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2013 OpenStack Foundation"},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_4d86ce14","line":1,"updated":"2015-12-10 02:36:49.000000000","message":"2015?","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"cf30973ff2eae0a3c041e72810518e263b6252ba","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2013 OpenStack Foundation"},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_23310eed","line":1,"in_reply_to":"7a740942_4d86ce14","updated":"2015-12-10 05:39:52.000000000","message":"ah, I will change to 2015, just copied from somewhere:)","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"e3e6de0755af46079c3d5d2d0f151adca6008bc3","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        self.mox.StubOutWithMock(self._session, \u0027call_xenapi\u0027)"},{"line_number":82,"context_line":"        self.mox.StubOutWithMock(self.bridge_driver, \u0027_ensure_vlan_bridge\u0027)"},{"line_number":83,"context_line":"        network \u003d Fake_Network()"},{"line_number":84,"context_line":"        self.mox.StubOutWithMock(network, \u0027get_meta\u0027)"},{"line_number":85,"context_line":"        network.get_meta(\u0027should_create_vlan\u0027).AndReturn(True)"},{"line_number":86,"context_line":"        vif \u003d Fake_Vif()"},{"line_number":87,"context_line":"        vif[\u0027network\u0027] \u003d network"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_133c19e1","line":84,"updated":"2015-12-07 10:58:14.000000000","message":"Since you mock out get_meta, is there a need to have a Fake_Network object?","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"424e620f832785e64bd4019f197a91e2e99c1b96","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        self.mox.StubOutWithMock(self._session, \u0027call_xenapi\u0027)"},{"line_number":82,"context_line":"        self.mox.StubOutWithMock(self.bridge_driver, \u0027_ensure_vlan_bridge\u0027)"},{"line_number":83,"context_line":"        network \u003d Fake_Network()"},{"line_number":84,"context_line":"        self.mox.StubOutWithMock(network, \u0027get_meta\u0027)"},{"line_number":85,"context_line":"        network.get_meta(\u0027should_create_vlan\u0027).AndReturn(True)"},{"line_number":86,"context_line":"        vif \u003d Fake_Vif()"},{"line_number":87,"context_line":"        vif[\u0027network\u0027] \u003d network"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_833d972c","line":84,"in_reply_to":"7a740942_133c19e1","updated":"2015-12-09 10:05:31.000000000","message":"yes, I will try to use the real network object and remove Fake_Network","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"e3e6de0755af46079c3d5d2d0f151adca6008bc3","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        network \u003d Fake_Network()"},{"line_number":84,"context_line":"        self.mox.StubOutWithMock(network, \u0027get_meta\u0027)"},{"line_number":85,"context_line":"        network.get_meta(\u0027should_create_vlan\u0027).AndReturn(True)"},{"line_number":86,"context_line":"        vif \u003d Fake_Vif()"},{"line_number":87,"context_line":"        vif[\u0027network\u0027] \u003d network"},{"line_number":88,"context_line":"        vif[\u0027address\u0027] \u003d \"fake_address\""},{"line_number":89,"context_line":"        network_ref \u003d \"fake_network_ref\""}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_d378d1c6","line":86,"updated":"2015-12-07 10:58:14.000000000","message":"Is nova.objects.virtual_interface.VirtualInterface.from_dict() better to use here?  Better to use the objects where we can, so changes in the object code themselves are also tested by these unit tests.","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"b0b9a82ac4b5561ca47d9a34229d1b547a413a7d","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        network \u003d Fake_Network()"},{"line_number":84,"context_line":"        self.mox.StubOutWithMock(network, \u0027get_meta\u0027)"},{"line_number":85,"context_line":"        network.get_meta(\u0027should_create_vlan\u0027).AndReturn(True)"},{"line_number":86,"context_line":"        vif \u003d Fake_Vif()"},{"line_number":87,"context_line":"        vif[\u0027network\u0027] \u003d network"},{"line_number":88,"context_line":"        vif[\u0027address\u0027] \u003d \"fake_address\""},{"line_number":89,"context_line":"        network_ref \u003d \"fake_network_ref\""}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_4ef7d38c","line":86,"in_reply_to":"7a740942_23fb8348","updated":"2015-12-09 11:10:39.000000000","message":"Perhaps the following?\n        vif \u003d vif_obj.VirtualInterface(context\u003dself.context)\n        vif.address \u003d \u0027fake_address\u0027\n        vif.network_id \u003d network","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"6f31f821d5ec9855a14a3effe6cf56c53e45db00","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        network \u003d Fake_Network()"},{"line_number":84,"context_line":"        self.mox.StubOutWithMock(network, \u0027get_meta\u0027)"},{"line_number":85,"context_line":"        network.get_meta(\u0027should_create_vlan\u0027).AndReturn(True)"},{"line_number":86,"context_line":"        vif \u003d Fake_Vif()"},{"line_number":87,"context_line":"        vif[\u0027network\u0027] \u003d network"},{"line_number":88,"context_line":"        vif[\u0027address\u0027] \u003d \"fake_address\""},{"line_number":89,"context_line":"        network_ref \u003d \"fake_network_ref\""}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_3c713ae1","line":86,"in_reply_to":"7a740942_4ef7d38c","updated":"2015-12-10 01:11:31.000000000","message":"yes, thanks very much","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"424e620f832785e64bd4019f197a91e2e99c1b96","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        network \u003d Fake_Network()"},{"line_number":84,"context_line":"        self.mox.StubOutWithMock(network, \u0027get_meta\u0027)"},{"line_number":85,"context_line":"        network.get_meta(\u0027should_create_vlan\u0027).AndReturn(True)"},{"line_number":86,"context_line":"        vif \u003d Fake_Vif()"},{"line_number":87,"context_line":"        vif[\u0027network\u0027] \u003d network"},{"line_number":88,"context_line":"        vif[\u0027address\u0027] \u003d \"fake_address\""},{"line_number":89,"context_line":"        network_ref \u003d \"fake_network_ref\""}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_23fb8348","line":86,"in_reply_to":"7a740942_d378d1c6","updated":"2015-12-09 10:05:31.000000000","message":"Got your concern, but I cannot find from_dict() in class VirtualInterface","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"cc0d1888ddfa42fda8e4a5fd4df7cfe4aa3e097a","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    def test_get_vif_ref_none(self):"},{"line_number":75,"context_line":"        vm_ref \u003d \"fake_vm_ref\""},{"line_number":76,"context_line":"        vif \u003d {\u0027address\u0027: \"fake_vif_address_none\"}"},{"line_number":77,"context_line":"        ret_vif_ref \u003d self.base_driver._get_vif_ref(vif, vm_ref)"},{"line_number":78,"context_line":"        self.assertIsNone(ret_vif_ref)"},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"7a5de9d1_c1f2a106","line":76,"updated":"2016-01-27 13:25:20.000000000","message":"missing_vif_address or no_match_vif_address","commit_id":"0fe910775cef17572b6b98ac681795d99ebcb347"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"c209f41f5d41c1dc00726077e59fa4a7328e7f50","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    def test_get_vif_ref_none(self):"},{"line_number":75,"context_line":"        vm_ref \u003d \"fake_vm_ref\""},{"line_number":76,"context_line":"        vif \u003d {\u0027address\u0027: \"fake_vif_address_none\"}"},{"line_number":77,"context_line":"        ret_vif_ref \u003d self.base_driver._get_vif_ref(vif, vm_ref)"},{"line_number":78,"context_line":"        self.assertIsNone(ret_vif_ref)"},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"7a5de9d1_e0545755","line":76,"in_reply_to":"7a5de9d1_c1f2a106","updated":"2016-01-28 02:00:01.000000000","message":"Done","commit_id":"0fe910775cef17572b6b98ac681795d99ebcb347"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"cc0d1888ddfa42fda8e4a5fd4df7cfe4aa3e097a","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    def test_create_vif_exception(self):"},{"line_number":96,"context_line":"        self.assertRaises(exception.NovaException,"},{"line_number":97,"context_line":"                          self.base_driver._create_vif,"},{"line_number":98,"context_line":"                          \"fake_vif\", \"exit\", \"fake_vm_ref\")"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    @mock.patch.object(vif.XenVIFDriver, \u0027_get_vif_ref\u0027,"},{"line_number":101,"context_line":"                       return_value\u003d\u0027fake_vif_ref\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"7a5de9d1_663f4326","line":98,"updated":"2016-01-27 13:25:20.000000000","message":"Instead of \u0027exit\u0027 can I suggest \"missing_vif_rec\", then it\u0027s clearer why this test is expecting to raise","commit_id":"0fe910775cef17572b6b98ac681795d99ebcb347"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"c209f41f5d41c1dc00726077e59fa4a7328e7f50","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    def test_create_vif_exception(self):"},{"line_number":96,"context_line":"        self.assertRaises(exception.NovaException,"},{"line_number":97,"context_line":"                          self.base_driver._create_vif,"},{"line_number":98,"context_line":"                          \"fake_vif\", \"exit\", \"fake_vm_ref\")"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    @mock.patch.object(vif.XenVIFDriver, \u0027_get_vif_ref\u0027,"},{"line_number":101,"context_line":"                       return_value\u003d\u0027fake_vif_ref\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"7a5de9d1_a04e5fbf","line":98,"in_reply_to":"7a5de9d1_663f4326","updated":"2016-01-28 02:00:01.000000000","message":"Done","commit_id":"0fe910775cef17572b6b98ac681795d99ebcb347"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"cc0d1888ddfa42fda8e4a5fd4df7cfe4aa3e097a","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        self.assertEqual(expected, self._session.call_xenapi.call_args_list)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    @mock.patch.object(vif.XenVIFDriver, \u0027_get_vif_ref\u0027,"},{"line_number":111,"context_line":"                       return_value\u003d\u0027fake_vif_ref_exception\u0027)"},{"line_number":112,"context_line":"    def test_unplug_exception(self, mock_get_vif_ref):"},{"line_number":113,"context_line":"        instance \u003d \"fake_instance\""},{"line_number":114,"context_line":"        vif \u003d \"fake_vif\""}],"source_content_type":"text/x-python","patch_set":14,"id":"7a5de9d1_26586b56","line":111,"updated":"2016-01-27 13:25:20.000000000","message":"Again, this could be \"missing_vif_ref\" as the return value","commit_id":"0fe910775cef17572b6b98ac681795d99ebcb347"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"c209f41f5d41c1dc00726077e59fa4a7328e7f50","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        self.assertEqual(expected, self._session.call_xenapi.call_args_list)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    @mock.patch.object(vif.XenVIFDriver, \u0027_get_vif_ref\u0027,"},{"line_number":111,"context_line":"                       return_value\u003d\u0027fake_vif_ref_exception\u0027)"},{"line_number":112,"context_line":"    def test_unplug_exception(self, mock_get_vif_ref):"},{"line_number":113,"context_line":"        instance \u003d \"fake_instance\""},{"line_number":114,"context_line":"        vif \u003d \"fake_vif\""}],"source_content_type":"text/x-python","patch_set":14,"id":"7a5de9d1_c04b5baf","line":111,"in_reply_to":"7a5de9d1_26586b56","updated":"2016-01-28 02:00:01.000000000","message":"Done","commit_id":"0fe910775cef17572b6b98ac681795d99ebcb347"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"cb2765c698b2028b508161b80ac76c8e1c712544","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self._session \u003d mock.Mock()"},{"line_number":49,"context_line":"        self._session.call_xenapi.side_effect \u003d fake_call_xenapi"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def tearDown(self):"},{"line_number":52,"context_line":"        super(XenVIFDriverTestBase, self).tearDown()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"class XenVIFDriverTestCase(XenVIFDriverTestBase):"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_eee765e2","line":52,"range":{"start_line":51,"start_character":4,"end_line":52,"end_character":52},"updated":"2016-02-20 00:41:18.000000000","message":"This is unnecessary","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"6f7b8deb52782ac32a2396a210100528755fb544","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self._session \u003d mock.Mock()"},{"line_number":49,"context_line":"        self._session.call_xenapi.side_effect \u003d fake_call_xenapi"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def tearDown(self):"},{"line_number":52,"context_line":"        super(XenVIFDriverTestBase, self).tearDown()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"class XenVIFDriverTestCase(XenVIFDriverTestBase):"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_38a33651","line":52,"range":{"start_line":51,"start_character":4,"end_line":52,"end_character":52},"in_reply_to":"bae84128_eee765e2","updated":"2016-02-21 09:06:51.000000000","message":"Done","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"cb2765c698b2028b508161b80ac76c8e1c712544","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        super(XenVIFDriverTestCase, self).setUp()"},{"line_number":58,"context_line":"        self.base_driver \u003d vif.XenVIFDriver(self._session)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def tearDown(self):"},{"line_number":61,"context_line":"        super(XenVIFDriverTestCase, self).tearDown()"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def test_get_vif_ref(self):"},{"line_number":64,"context_line":"        vm_ref \u003d \"fake_vm_ref\""}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_8ee219d4","line":61,"range":{"start_line":60,"start_character":4,"end_line":61,"end_character":52},"updated":"2016-02-20 00:41:18.000000000","message":"This is unnecessary","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"6f7b8deb52782ac32a2396a210100528755fb544","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        super(XenVIFDriverTestCase, self).setUp()"},{"line_number":58,"context_line":"        self.base_driver \u003d vif.XenVIFDriver(self._session)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def tearDown(self):"},{"line_number":61,"context_line":"        super(XenVIFDriverTestCase, self).tearDown()"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def test_get_vif_ref(self):"},{"line_number":64,"context_line":"        vm_ref \u003d \"fake_vm_ref\""}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_988a42be","line":61,"range":{"start_line":60,"start_character":4,"end_line":61,"end_character":52},"in_reply_to":"bae84128_8ee219d4","updated":"2016-02-21 09:06:51.000000000","message":"Done","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"cb2765c698b2028b508161b80ac76c8e1c712544","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        self.assertRaises(exception.NovaException,"},{"line_number":117,"context_line":"                          self.base_driver.unplug,"},{"line_number":118,"context_line":"                          instance, vif, vm_ref)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"class XenAPIBridgeDriverTestCase(XenVIFDriverTestBase, object):"},{"line_number":122,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_ae955de5","line":119,"updated":"2016-02-20 00:41:18.000000000","message":"It looks like we\u0027re missing a test for _get_vif_ref with an exception raised that verifies it continues to the next vif and returns it","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"6f7b8deb52782ac32a2396a210100528755fb544","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        self.assertRaises(exception.NovaException,"},{"line_number":117,"context_line":"                          self.base_driver.unplug,"},{"line_number":118,"context_line":"                          instance, vif, vm_ref)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"class XenAPIBridgeDriverTestCase(XenVIFDriverTestBase, object):"},{"line_number":122,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_d894cae3","line":119,"in_reply_to":"bae84128_ae955de5","updated":"2016-02-21 09:06:51.000000000","message":"yes, I will add a test for exception scenario, thanks:)","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"cb2765c698b2028b508161b80ac76c8e1c712544","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        super(XenAPIBridgeDriverTestCase, self).setUp()"},{"line_number":124,"context_line":"        self.bridge_driver \u003d vif.XenAPIBridgeDriver(self._session)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def tearDown(self):"},{"line_number":127,"context_line":"        super(XenAPIBridgeDriverTestCase, self).tearDown()"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    @mock.patch.object(vif.XenAPIBridgeDriver, \u0027_ensure_vlan_bridge\u0027,"},{"line_number":130,"context_line":"                       return_value\u003d\u0027fake_network_ref\u0027)"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_6e4a35c7","line":127,"range":{"start_line":126,"start_character":4,"end_line":127,"end_character":58},"updated":"2016-02-20 00:41:18.000000000","message":"This is unnecessary","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"6f7b8deb52782ac32a2396a210100528755fb544","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        super(XenAPIBridgeDriverTestCase, self).setUp()"},{"line_number":124,"context_line":"        self.bridge_driver \u003d vif.XenAPIBridgeDriver(self._session)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def tearDown(self):"},{"line_number":127,"context_line":"        super(XenAPIBridgeDriverTestCase, self).tearDown()"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    @mock.patch.object(vif.XenAPIBridgeDriver, \u0027_ensure_vlan_bridge\u0027,"},{"line_number":130,"context_line":"                       return_value\u003d\u0027fake_network_ref\u0027)"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_b88f46cb","line":127,"range":{"start_line":126,"start_character":4,"end_line":127,"end_character":58},"in_reply_to":"bae84128_6e4a35c7","updated":"2016-02-21 09:06:51.000000000","message":"Done","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"cb2765c698b2028b508161b80ac76c8e1c712544","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        device \u003d 1"},{"line_number":143,"context_line":"        ret_vif_ref \u003d self.bridge_driver.plug(instance, vif, vm_ref, device)"},{"line_number":144,"context_line":"        self.assertEqual(\u0027fake_vif_ref\u0027, ret_vif_ref)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase):"},{"line_number":148,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_0e6ba91f","line":145,"updated":"2016-02-20 00:41:18.000000000","message":"It looks like we\u0027re missing a test for unplug for the XenAPIBridgeDriver?","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"6f7b8deb52782ac32a2396a210100528755fb544","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        device \u003d 1"},{"line_number":143,"context_line":"        ret_vif_ref \u003d self.bridge_driver.plug(instance, vif, vm_ref, device)"},{"line_number":144,"context_line":"        self.assertEqual(\u0027fake_vif_ref\u0027, ret_vif_ref)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase):"},{"line_number":148,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_184bd288","line":145,"in_reply_to":"bae84128_0e6ba91f","updated":"2016-02-21 09:06:51.000000000","message":"Ah, I can add a test for unplug. \nunplug in XenAPIBridgeDriver is totally inherited from XenVIFDriver, so I didn\u0027t add a test for it since there was a test for the function in parent class.\nWe add unplug() in the child functions mainly to make the framework of vif.py and these functions will be re-written later in other commit","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"fc03676992a5fed4c88e4c91e5fc7898f57bf703","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        device \u003d 1"},{"line_number":143,"context_line":"        ret_vif_ref \u003d self.bridge_driver.plug(instance, vif, vm_ref, device)"},{"line_number":144,"context_line":"        self.assertEqual(\u0027fake_vif_ref\u0027, ret_vif_ref)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase):"},{"line_number":148,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_1c51d40e","line":145,"in_reply_to":"bae84128_184bd288","updated":"2016-02-22 18:11:05.000000000","message":"Okay. The same is true for XenAPIOpenVswitchDriver and you had added an unplug test there. Also, I thought there\u0027s some merit in testing to ensure future changes to unplug don\u0027t cause unexpected behavior.","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"cb2765c698b2028b508161b80ac76c8e1c712544","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        super(XenAPIOpenVswitchDriverTestCase, self).setUp()"},{"line_number":150,"context_line":"        self.ovs_driver \u003d vif.XenAPIOpenVswitchDriver(self._session)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def tearDown(self):"},{"line_number":153,"context_line":"        super(XenAPIOpenVswitchDriverTestCase, self).tearDown()"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    @mock.patch.object(network_utils, \u0027find_network_with_bridge\u0027,"},{"line_number":156,"context_line":"                       return_value\u003d\u0027fake_network_ref\u0027)"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_ae127db2","line":153,"range":{"start_line":152,"start_character":4,"end_line":153,"end_character":63},"updated":"2016-02-20 00:41:18.000000000","message":"This is unnecessary","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"6f7b8deb52782ac32a2396a210100528755fb544","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        super(XenAPIOpenVswitchDriverTestCase, self).setUp()"},{"line_number":150,"context_line":"        self.ovs_driver \u003d vif.XenAPIOpenVswitchDriver(self._session)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def tearDown(self):"},{"line_number":153,"context_line":"        super(XenAPIOpenVswitchDriverTestCase, self).tearDown()"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    @mock.patch.object(network_utils, \u0027find_network_with_bridge\u0027,"},{"line_number":156,"context_line":"                       return_value\u003d\u0027fake_network_ref\u0027)"}],"source_content_type":"text/x-python","patch_set":17,"id":"bae84128_18bd9268","line":153,"range":{"start_line":152,"start_character":4,"end_line":153,"end_character":63},"in_reply_to":"bae84128_ae127db2","updated":"2016-02-21 09:06:51.000000000","message":"Done","commit_id":"38b5dd7a478edac5a4cff9f310436d74e852c21d"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"ac6bbe3248dafdb1491e1fd021ae98c3d257adee","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                       return_value\u003d\u0027fake_vif_ref\u0027)"},{"line_number":158,"context_line":"    def test_unplug(self, mock_get_vif_ref):"},{"line_number":159,"context_line":"        instance \u003d {\u0027name\u0027: \"fake_instance\"}"},{"line_number":160,"context_line":"        vif \u003d \"fake_vif\""},{"line_number":161,"context_line":"        vm_ref \u003d \"fake_vm_ref\""},{"line_number":162,"context_line":"        self.bridge_driver.unplug(instance, vif, vm_ref)"},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"bae84128_dfeae5ab","line":160,"updated":"2016-02-22 02:17:55.000000000","message":"if there is another version of update, could you help to use the newly defined structure of fake_vif instead of this string of \u0027fake_vif\u0027?","commit_id":"c4948c9bb2648d3b671452766f50519b55b8b8c9"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"bd6bb5d585fe7ce3bd34a000b99820f85a1a280e","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                       return_value\u003d\u0027fake_vif_ref\u0027)"},{"line_number":158,"context_line":"    def test_unplug(self, mock_get_vif_ref):"},{"line_number":159,"context_line":"        instance \u003d {\u0027name\u0027: \"fake_instance\"}"},{"line_number":160,"context_line":"        vif \u003d \"fake_vif\""},{"line_number":161,"context_line":"        vm_ref \u003d \"fake_vm_ref\""},{"line_number":162,"context_line":"        self.bridge_driver.unplug(instance, vif, vm_ref)"},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"bae84128_743bfbd1","line":160,"in_reply_to":"bae84128_dfeae5ab","updated":"2016-02-22 03:22:20.000000000","message":"yes, will push a new patch","commit_id":"c4948c9bb2648d3b671452766f50519b55b8b8c9"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"cbb6b9c279d568367940f418435476a27900bba0","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        super(XenAPIOpenVswitchDriverTestCase, self).setUp()"},{"line_number":166,"context_line":"        self.ovs_driver \u003d vif.XenAPIOpenVswitchDriver(self._session)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def tearDown(self):"},{"line_number":169,"context_line":"        super(XenAPIOpenVswitchDriverTestCase, self).tearDown()"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    @mock.patch.object(network_utils, \u0027find_network_with_bridge\u0027,"},{"line_number":172,"context_line":"                       return_value\u003d\u0027fake_network_ref\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"bae84128_68bb1728","line":169,"range":{"start_line":168,"start_character":4,"end_line":169,"end_character":63},"updated":"2016-02-22 09:48:43.000000000","message":"Melanie requested this was removed.","commit_id":"a1f50e14fc6ea6c0141cc3cb6a9b9a5d7a207a27"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d892b5a6ed58dd5550b4227cee2fc1bfa195f9c3","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        expected \u003d [mock.call(\u0027VM.get_VIFs\u0027, vm_ref),"},{"line_number":93,"context_line":"                    mock.call(\u0027VIF.get_record\u0027, \u0027fake_vif_ref\u0027),"},{"line_number":94,"context_line":"                    mock.call(\u0027VIF.get_record\u0027, \u0027fake_vif_ref_A2\u0027)]"},{"line_number":95,"context_line":"        self.assertEqual(expected, self._session.call_xenapi.call_args_list)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def test_create_vif(self):"},{"line_number":98,"context_line":"        vif_rec \u003d \"fake_vif_rec\""}],"source_content_type":"text/x-python","patch_set":20,"id":"bae84128_f03250f1","line":95,"updated":"2016-02-22 19:35:50.000000000","message":"When I saw you changed the name of this test without changing the content, I went and had a second look through to find how the exception is raised.\n\nSo this test has a vif that doesn\u0027t match during the first loop iteration, then raises an exception the second iteration, which is caught and continues on to return None at the end. Sorry I missed it.","commit_id":"0e3372ac2e9444e69fde6df14a445ff1b963988a"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"e8bbc891d6dcf689c514e5065e8f28a9ca0e8397","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        expected \u003d [mock.call(\u0027VM.get_VIFs\u0027, vm_ref),"},{"line_number":93,"context_line":"                    mock.call(\u0027VIF.get_record\u0027, \u0027fake_vif_ref\u0027),"},{"line_number":94,"context_line":"                    mock.call(\u0027VIF.get_record\u0027, \u0027fake_vif_ref_A2\u0027)]"},{"line_number":95,"context_line":"        self.assertEqual(expected, self._session.call_xenapi.call_args_list)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def test_create_vif(self):"},{"line_number":98,"context_line":"        vif_rec \u003d \"fake_vif_rec\""}],"source_content_type":"text/x-python","patch_set":20,"id":"bae84128_06721349","line":95,"in_reply_to":"bae84128_f03250f1","updated":"2016-02-23 01:15:48.000000000","message":"yes, it\u0027s just my meaning on this test, thanks a lot:)","commit_id":"0e3372ac2e9444e69fde6df14a445ff1b963988a"}],"nova/virt/xenapi/vif.py":[{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"64fdf4f4c7d5bfaf89767fe0de95283230949fd3","unresolved":false,"context_lines":[{"line_number":153,"context_line":"            self._session.call_xenapi(\u0027VIF.unplug\u0027, vif_ref)"},{"line_number":154,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"},{"line_number":155,"context_line":"        except Exception:"},{"line_number":156,"context_line":"            LOG.debug(\"Fail to unplug vif %s\", vif, instance\u003dinstance)"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"class XenAPIOpenVswitchDriver(XenVIFDriver):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_cdeddef0","line":156,"updated":"2015-12-10 02:36:49.000000000","message":"nit: suggest add vif_ref in the debug message for trouble-shooting.","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"cf30973ff2eae0a3c041e72810518e263b6252ba","unresolved":false,"context_lines":[{"line_number":153,"context_line":"            self._session.call_xenapi(\u0027VIF.unplug\u0027, vif_ref)"},{"line_number":154,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"},{"line_number":155,"context_line":"        except Exception:"},{"line_number":156,"context_line":"            LOG.debug(\"Fail to unplug vif %s\", vif, instance\u003dinstance)"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"class XenAPIOpenVswitchDriver(XenVIFDriver):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_a3f73ef4","line":156,"in_reply_to":"7a740942_cdeddef0","updated":"2015-12-10 05:39:52.000000000","message":"understood, I didn\u0027t add vif_ref just in case there is no vif_ref found after calling self._get_vif_ref(vif, vm_ref), if use vif_ref in debug log at this time, there will be a new exception raised","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"64fdf4f4c7d5bfaf89767fe0de95283230949fd3","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        except Exception:"},{"line_number":189,"context_line":"            # If create VIF meet exception, maybe the VIF already exists,"},{"line_number":190,"context_line":"            # skip the exception and try to find the VIF"},{"line_number":191,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":192,"context_line":"        finally:"},{"line_number":193,"context_line":"            return vif_ref"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_2dc902ad","line":191,"updated":"2015-12-10 02:36:49.000000000","message":"How will it go if got no vif_ref? e.g. VIF.create failed due to some reasons other than \"VIF already existing\"?","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"cf30973ff2eae0a3c041e72810518e263b6252ba","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        except Exception:"},{"line_number":189,"context_line":"            # If create VIF meet exception, maybe the VIF already exists,"},{"line_number":190,"context_line":"            # skip the exception and try to find the VIF"},{"line_number":191,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":192,"context_line":"        finally:"},{"line_number":193,"context_line":"            return vif_ref"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_437c5a20","line":191,"in_reply_to":"7a740942_2dc902ad","updated":"2015-12-10 05:39:52.000000000","message":"right, I will check whether vif_ref is None, if still so, will raise an exception","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"64fdf4f4c7d5bfaf89767fe0de95283230949fd3","unresolved":false,"context_lines":[{"line_number":190,"context_line":"            # skip the exception and try to find the VIF"},{"line_number":191,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":192,"context_line":"        finally:"},{"line_number":193,"context_line":"            return vif_ref"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def unplug(self, instance, vif):"},{"line_number":196,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_ed945a45","line":193,"updated":"2015-12-10 02:36:49.000000000","message":"in the old vmops.py, there is debug log to record the vif_ref. can we add similar debug message at here?","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"cf30973ff2eae0a3c041e72810518e263b6252ba","unresolved":false,"context_lines":[{"line_number":190,"context_line":"            # skip the exception and try to find the VIF"},{"line_number":191,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":192,"context_line":"        finally:"},{"line_number":193,"context_line":"            return vif_ref"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def unplug(self, instance, vif):"},{"line_number":196,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_6379562f","line":193,"in_reply_to":"7a740942_ed945a45","updated":"2015-12-10 05:39:52.000000000","message":"sure, can add debug log","commit_id":"736b777a4443ea3500411e7959b834e9f924cffd"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"5d4d62bc6d3e317856844cc136beed1dba91646c","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def unplug(self, instance, vif):"},{"line_number":157,"context_line":"        try:"},{"line_number":158,"context_line":"            vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":159,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":160,"context_line":"            self._session.call_xenapi(\u0027VIF.unplug\u0027, vif_ref)"},{"line_number":161,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a710552_c40ced44","line":158,"updated":"2015-12-15 03:07:50.000000000","message":"currently vm_destroy goes ahead before vif.unplug; so these code block may not take effective at all as vif has been destroyed along with VM destroy.","commit_id":"a1aa9883727a6a03dc709a88d30c95f1718e830b"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"450caeeb8565dcb6ddf734536dbf4dec8f878bf0","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def unplug(self, instance, vif):"},{"line_number":157,"context_line":"        try:"},{"line_number":158,"context_line":"            vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":159,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":160,"context_line":"            self._session.call_xenapi(\u0027VIF.unplug\u0027, vif_ref)"},{"line_number":161,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a710552_ff04d47e","line":158,"in_reply_to":"5a710552_c40ced44","updated":"2015-12-16 01:56:16.000000000","message":"yes, will change the call order in upper function vmops.py","commit_id":"a1aa9883727a6a03dc709a88d30c95f1718e830b"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"1c8df54541cbea8a10b1091f1dc08b0e550b586a","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        try:"},{"line_number":158,"context_line":"            vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":159,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":160,"context_line":"            self._session.call_xenapi(\u0027VIF.unplug\u0027, vif_ref)"},{"line_number":161,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"},{"line_number":162,"context_line":"        except Exception:"},{"line_number":163,"context_line":"            LOG.debug(\"Fail to unplug vif %s\", vif, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a710552_6dedb68a","line":160,"updated":"2015-12-15 16:10:48.000000000","message":"We need to call vm_destroy _after_ the vif unplugging and, remove the VIF.unplug line above.\n\nThe VIF.unplug call only makes sense for vif hotplug/unplug, and that\u0027s a separate change which shouldn\u0027t be included in this bugfix.\n\nCan this code be a bit more robust as well?  if vm_ref is None we can return, if vif_ref is None, we can return (as in either case there actually is nothing to do)","commit_id":"a1aa9883727a6a03dc709a88d30c95f1718e830b"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"450caeeb8565dcb6ddf734536dbf4dec8f878bf0","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        try:"},{"line_number":158,"context_line":"            vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":159,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":160,"context_line":"            self._session.call_xenapi(\u0027VIF.unplug\u0027, vif_ref)"},{"line_number":161,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"},{"line_number":162,"context_line":"        except Exception:"},{"line_number":163,"context_line":"            LOG.debug(\"Fail to unplug vif %s\", vif, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a710552_9f3b5038","line":160,"in_reply_to":"5a710552_6dedb68a","updated":"2015-12-16 01:56:16.000000000","message":"yes, will change this part. I misunderstood VIF.unplug before, it\u0027s only used for hot unplug:)","commit_id":"a1aa9883727a6a03dc709a88d30c95f1718e830b"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"1c8df54541cbea8a10b1091f1dc08b0e550b586a","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        try:"},{"line_number":195,"context_line":"            vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":196,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":197,"context_line":"            self._session.call_xenapi(\u0027VIF.unplug\u0027, vif_ref)"},{"line_number":198,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"},{"line_number":199,"context_line":"        except Exception:"},{"line_number":200,"context_line":"            LOG.debug(\"Fail to unplug vif %s\", vif, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a710552_ad9abe12","line":197,"updated":"2015-12-15 16:10:48.000000000","message":"Ditto.","commit_id":"a1aa9883727a6a03dc709a88d30c95f1718e830b"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"1f4526bdfda44fd84026dcf75a7e17c6f2a119be","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            vif_ref \u003d self._session.call_xenapi(\u0027VIF.create\u0027, vif_rec)"},{"line_number":54,"context_line":"        except Exception:"},{"line_number":55,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":56,"context_line":"            if vif_ref:"},{"line_number":57,"context_line":"                return vif_ref"},{"line_number":58,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a7e1126_9787d80e","line":55,"updated":"2015-12-17 12:06:21.000000000","message":"This can\u0027t happen; if xenapi call fails then we should just raise the NovaException.  Sorry for not spotting this earlier.","commit_id":"94b31f47659f7be45f7d5fe84c3c73cb105c207f"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"468dc89bbf69bc49ae75eb722ccb1f7d12e8217a","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            vif_ref \u003d self._session.call_xenapi(\u0027VIF.create\u0027, vif_rec)"},{"line_number":54,"context_line":"        except Exception:"},{"line_number":55,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":56,"context_line":"            if vif_ref:"},{"line_number":57,"context_line":"                return vif_ref"},{"line_number":58,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a7e1126_8f61d23b","line":55,"in_reply_to":"3a7e1126_24e10ad1","updated":"2015-12-18 01:46:55.000000000","message":"yes, it\u0027s much reasonable for checking vif\u0027s existance first.\nI also thought of this during implementing this part, just in most of case, VIF is not exist, so calling checking the existance will only waste.\nBut it is really logical clear for checking VIF\u0027s existance first, I will change this part","commit_id":"94b31f47659f7be45f7d5fe84c3c73cb105c207f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"e34442ab8b413cfefa095c5771389cfd1fa7fe5d","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            vif_ref \u003d self._session.call_xenapi(\u0027VIF.create\u0027, vif_rec)"},{"line_number":54,"context_line":"        except Exception:"},{"line_number":55,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":56,"context_line":"            if vif_ref:"},{"line_number":57,"context_line":"                return vif_ref"},{"line_number":58,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a7e1126_24e10ad1","line":55,"in_reply_to":"3a7e1126_8ea1feab","updated":"2015-12-17 13:46:26.000000000","message":"Understood - in which case, let\u0027s get_vif_ref first and return that if it already exists (i.e. there is nothing to do).\n\nI think the check for the vif already existing should be in plug() after we get the vm_ref.  One reason for this is that if the record is _different_ for some reason, then a caller might assume that XenAPI might update an existing vif?  Thoughts?","commit_id":"94b31f47659f7be45f7d5fe84c3c73cb105c207f"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"c2523082bcfeff0a9b11f02041d1f832202f9c71","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            vif_ref \u003d self._session.call_xenapi(\u0027VIF.create\u0027, vif_rec)"},{"line_number":54,"context_line":"        except Exception:"},{"line_number":55,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":56,"context_line":"            if vif_ref:"},{"line_number":57,"context_line":"                return vif_ref"},{"line_number":58,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a7e1126_8ea1feab","line":55,"in_reply_to":"3a7e1126_9787d80e","updated":"2015-12-17 13:40:26.000000000","message":"This part is not really meant for VIF.create failed scenario, if the vif is already exist, calling VIF.create will got exception. When our nova-compute service restart, it will check the already existed instance and do vif.plug, see compute/manager.py. \nBefore this change, previous plug does nothing and there is no problem before.\nNow when VIF.create got exception, the code will first check whether the vif is really exist, if not, return None will let the upper part know and do some exception work like what it do before","commit_id":"94b31f47659f7be45f7d5fe84c3c73cb105c207f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"1f4526bdfda44fd84026dcf75a7e17c6f2a119be","unresolved":false,"context_lines":[{"line_number":154,"context_line":"        return network_ref"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def unplug(self, instance, vif):"},{"line_number":157,"context_line":"        try:"},{"line_number":158,"context_line":"            vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":159,"context_line":"            if not vm_ref:"},{"line_number":160,"context_line":"                LOG.debug(\"vm didn\u0027t exist, no need to unplug vif %s\","}],"source_content_type":"text/x-python","patch_set":6,"id":"3a7e1126_975d98eb","line":157,"updated":"2015-12-17 12:06:21.000000000","message":"This code is common between the drivers so should be on the parent class to avoid duplication","commit_id":"94b31f47659f7be45f7d5fe84c3c73cb105c207f"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"c2523082bcfeff0a9b11f02041d1f832202f9c71","unresolved":false,"context_lines":[{"line_number":154,"context_line":"        return network_ref"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def unplug(self, instance, vif):"},{"line_number":157,"context_line":"        try:"},{"line_number":158,"context_line":"            vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":159,"context_line":"            if not vm_ref:"},{"line_number":160,"context_line":"                LOG.debug(\"vm didn\u0027t exist, no need to unplug vif %s\","}],"source_content_type":"text/x-python","patch_set":6,"id":"3a7e1126_93c7e511","line":157,"in_reply_to":"3a7e1126_975d98eb","updated":"2015-12-17 13:40:26.000000000","message":"yes, sorry for missing this part, this should be the common one","commit_id":"94b31f47659f7be45f7d5fe84c3c73cb105c207f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"1b8aa8d2e901c87d9a08f1771dd63f58eba776dc","unresolved":false,"context_lines":[{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        return network_ref"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def unplug(self, instance, vif):"},{"line_number":130,"context_line":"        pass"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a7e1126_a6f057ff","side":"PARENT","line":129,"updated":"2015-12-18 14:52:39.000000000","message":"I think we need to keep this and call the parent\u0027s unplug, rather than just remove it from here.\n\nI know object inheritance will take care of it for us, but in a future change we\u0027ll need to re-add these direct unplug methods as well.","commit_id":"236bf85b2eaa19ab132ba1dad7be9d91f2dc3898"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"7252fdfc96c95beb6ef8e90835de5c77277e34ef","unresolved":false,"context_lines":[{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        return network_ref"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def unplug(self, instance, vif):"},{"line_number":130,"context_line":"        pass"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a7e1126_6151ce35","side":"PARENT","line":129,"in_reply_to":"3a7e1126_a6f057ff","updated":"2015-12-21 02:06:31.000000000","message":"Done","commit_id":"236bf85b2eaa19ab132ba1dad7be9d91f2dc3898"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"17672bca47c96f09226b65fd47ac83a69e0fad91","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def unplug(self, instance, vif):"},{"line_number":63,"context_line":"        try:"},{"line_number":64,"context_line":"            vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":65,"context_line":"            if not vm_ref:"},{"line_number":66,"context_line":"                LOG.debug(\"vm didn\u0027t exist, no need to unplug vif %s\","},{"line_number":67,"context_line":"                        vif, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_7f8150fa","line":64,"updated":"2016-01-06 10:08:10.000000000","message":"we cannot use instance[\u0027name\u0027] directly because when resizing,  source_instance and dest_instance have the same instance[\u0027name\u0027],\nif we use this, we will unplug the dest_instance\u0027s vif which is not what we really want do","commit_id":"5cada72b41f2ad1b7468c19f63abf860eb5de785"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3d67ca5256d1199c75aa5c3aeb577de08668612c","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        vif_refs \u003d self._session.call_xenapi(\"VM.get_VIFs\", vm_ref)"},{"line_number":45,"context_line":"        for vif_ref in vif_refs:"},{"line_number":46,"context_line":"            vif_rec \u003d self._session.call_xenapi(\u0027VIF.get_record\u0027, vif_ref)"},{"line_number":47,"context_line":"            if vif_rec[\u0027MAC\u0027] \u003d\u003d vif[\u0027address\u0027]:"},{"line_number":48,"context_line":"                return vif_ref"},{"line_number":49,"context_line":"        return None"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_3b5313e4","line":47,"range":{"start_line":47,"start_character":33,"end_line":47,"end_character":47},"updated":"2016-01-14 11:40:14.000000000","message":"Hmm, thinking about using the MAC address, this doesn\u0027t feel quite right, but its probably the best way to do this.","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3d67ca5256d1199c75aa5c3aeb577de08668612c","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        vif_refs \u003d self._session.call_xenapi(\"VM.get_VIFs\", vm_ref)"},{"line_number":45,"context_line":"        for vif_ref in vif_refs:"},{"line_number":46,"context_line":"            vif_rec \u003d self._session.call_xenapi(\u0027VIF.get_record\u0027, vif_ref)"},{"line_number":47,"context_line":"            if vif_rec[\u0027MAC\u0027] \u003d\u003d vif[\u0027address\u0027]:"},{"line_number":48,"context_line":"                return vif_ref"},{"line_number":49,"context_line":"        return None"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_b83869ef","line":47,"updated":"2016-01-14 11:40:14.000000000","message":"Nit: we should probably handle the case were a vif is removed while we are look around our list of VIFs.\n\nSomething like this:\nhttps://github.com/openstack/nova/blob/master/nova/virt/xenapi/vm_utils.py#L1976\n\nIn this case I think we just need:\n  if vif_ref and vif_rec[\u0027MAC\u0027] \u003d\u003d vif[\u0027address\u0027]:\n\nNow in theory we should never hit that here, but its good to be defensive I feel.","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"14255ca75a9874d36cdf0e166ba21ac18e3ea779","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        vif_refs \u003d self._session.call_xenapi(\"VM.get_VIFs\", vm_ref)"},{"line_number":45,"context_line":"        for vif_ref in vif_refs:"},{"line_number":46,"context_line":"            vif_rec \u003d self._session.call_xenapi(\u0027VIF.get_record\u0027, vif_ref)"},{"line_number":47,"context_line":"            if vif_rec[\u0027MAC\u0027] \u003d\u003d vif[\u0027address\u0027]:"},{"line_number":48,"context_line":"                return vif_ref"},{"line_number":49,"context_line":"        return None"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_cf1be47f","line":47,"range":{"start_line":47,"start_character":33,"end_line":47,"end_character":47},"in_reply_to":"da6ed579_3b5313e4","updated":"2016-01-15 02:57:08.000000000","message":"yes, we cannot find other params to be used","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"14255ca75a9874d36cdf0e166ba21ac18e3ea779","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        vif_refs \u003d self._session.call_xenapi(\"VM.get_VIFs\", vm_ref)"},{"line_number":45,"context_line":"        for vif_ref in vif_refs:"},{"line_number":46,"context_line":"            vif_rec \u003d self._session.call_xenapi(\u0027VIF.get_record\u0027, vif_ref)"},{"line_number":47,"context_line":"            if vif_rec[\u0027MAC\u0027] \u003d\u003d vif[\u0027address\u0027]:"},{"line_number":48,"context_line":"                return vif_ref"},{"line_number":49,"context_line":"        return None"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_ef6b47b5","line":47,"in_reply_to":"da6ed579_5ec35557","updated":"2016-01-15 02:57:08.000000000","message":"agree, will change this","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"8fe0102b6c671176eedfa9abb85541c2287fe987","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        vif_refs \u003d self._session.call_xenapi(\"VM.get_VIFs\", vm_ref)"},{"line_number":45,"context_line":"        for vif_ref in vif_refs:"},{"line_number":46,"context_line":"            vif_rec \u003d self._session.call_xenapi(\u0027VIF.get_record\u0027, vif_ref)"},{"line_number":47,"context_line":"            if vif_rec[\u0027MAC\u0027] \u003d\u003d vif[\u0027address\u0027]:"},{"line_number":48,"context_line":"                return vif_ref"},{"line_number":49,"context_line":"        return None"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_5ec35557","line":47,"in_reply_to":"da6ed579_b83869ef","updated":"2016-01-14 12:01:24.000000000","message":"I think this raises HANDLE_INVALID.\nIn this case therefore catch the exception and ignore the vif_ref, moving on to the next one.","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"14255ca75a9874d36cdf0e166ba21ac18e3ea779","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        vif_refs \u003d self._session.call_xenapi(\"VM.get_VIFs\", vm_ref)"},{"line_number":45,"context_line":"        for vif_ref in vif_refs:"},{"line_number":46,"context_line":"            vif_rec \u003d self._session.call_xenapi(\u0027VIF.get_record\u0027, vif_ref)"},{"line_number":47,"context_line":"            if vif_rec[\u0027MAC\u0027] \u003d\u003d vif[\u0027address\u0027]:"},{"line_number":48,"context_line":"                return vif_ref"},{"line_number":49,"context_line":"        return None"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_6f7757a0","line":47,"in_reply_to":"da6ed579_b83869ef","updated":"2016-01-15 02:57:08.000000000","message":"yes, I should handle the scenario you pointed. I agree with Bob\u0027s suggestion, vif_ref list has been accquired firstly, so each vif_ref itself should not be None, but as you said the vif_ref maybe actually meaningless since it was removed while we are loop the list.","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"8fe0102b6c671176eedfa9abb85541c2287fe987","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    def _create_vif(self, vif, vif_rec, vm_ref):"},{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            vif_ref \u003d self._session.call_xenapi(\u0027VIF.create\u0027, vif_rec)"},{"line_number":54,"context_line":"        except Exception:"},{"line_number":55,"context_line":"            raise exception.NovaException("},{"line_number":56,"context_line":"                reason\u003d_(\"Failed to create vif %s\") % vif)"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_7ea5f99f","line":54,"updated":"2016-01-14 12:01:24.000000000","message":"Can we log the XAPI exception please","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"14255ca75a9874d36cdf0e166ba21ac18e3ea779","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    def _create_vif(self, vif, vif_rec, vm_ref):"},{"line_number":52,"context_line":"        try:"},{"line_number":53,"context_line":"            vif_ref \u003d self._session.call_xenapi(\u0027VIF.create\u0027, vif_rec)"},{"line_number":54,"context_line":"        except Exception:"},{"line_number":55,"context_line":"            raise exception.NovaException("},{"line_number":56,"context_line":"                reason\u003d_(\"Failed to create vif %s\") % vif)"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_ef37c8fc","line":54,"in_reply_to":"da6ed579_7ea5f99f","updated":"2016-01-15 02:57:08.000000000","message":"Done","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3d67ca5256d1199c75aa5c3aeb577de08668612c","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    def unplug(self, instance, vif, vm_ref\u003dNone):"},{"line_number":63,"context_line":"        try:"},{"line_number":64,"context_line":"            if not vm_ref:"},{"line_number":65,"context_line":"                vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            LOG.debug(\"unplug vif, vif:%(vif)s, vm_ref:%(vm_ref)s\","},{"line_number":68,"context_line":"                      {\u0027vif\u0027: vif, \u0027vm_ref\u0027: vm_ref}, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_9b123f56","line":65,"updated":"2016-01-14 11:40:14.000000000","message":"Nit: I think this sits better in the vmops method that calls into here, and so make vm_ref a required arg.","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"8fe0102b6c671176eedfa9abb85541c2287fe987","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    def unplug(self, instance, vif, vm_ref\u003dNone):"},{"line_number":63,"context_line":"        try:"},{"line_number":64,"context_line":"            if not vm_ref:"},{"line_number":65,"context_line":"                vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            LOG.debug(\"unplug vif, vif:%(vif)s, vm_ref:%(vm_ref)s\","},{"line_number":68,"context_line":"                      {\u0027vif\u0027: vif, \u0027vm_ref\u0027: vm_ref}, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_bea8518d","line":65,"in_reply_to":"da6ed579_9b123f56","updated":"2016-01-14 12:01:24.000000000","message":"Agreed.","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"14255ca75a9874d36cdf0e166ba21ac18e3ea779","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    def unplug(self, instance, vif, vm_ref\u003dNone):"},{"line_number":63,"context_line":"        try:"},{"line_number":64,"context_line":"            if not vm_ref:"},{"line_number":65,"context_line":"                vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            LOG.debug(\"unplug vif, vif:%(vif)s, vm_ref:%(vm_ref)s\","},{"line_number":68,"context_line":"                      {\u0027vif\u0027: vif, \u0027vm_ref\u0027: vm_ref}, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_8fd08cca","line":65,"in_reply_to":"da6ed579_9b123f56","updated":"2016-01-15 02:57:08.000000000","message":"agreed, will change this","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3d67ca5256d1199c75aa5c3aeb577de08668612c","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"},{"line_number":75,"context_line":"        except Exception as e:"},{"line_number":76,"context_line":"            LOG.debug(\"Fail to unplug vif %(vif)s, exception:%(exception)s\","},{"line_number":77,"context_line":"                      {\u0027vif\u0027: vif, \u0027exception\u0027: e}, instance\u003dinstance)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"class XenAPIBridgeDriver(XenVIFDriver):"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_8fd47020","line":77,"updated":"2016-01-14 11:40:14.000000000","message":"Should we not error out on a failed unplug? Rather than fail silently?\n\nNot against add a log message here, but feels like we should reraise the exception.","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"8fe0102b6c671176eedfa9abb85541c2287fe987","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"},{"line_number":75,"context_line":"        except Exception as e:"},{"line_number":76,"context_line":"            LOG.debug(\"Fail to unplug vif %(vif)s, exception:%(exception)s\","},{"line_number":77,"context_line":"                      {\u0027vif\u0027: vif, \u0027exception\u0027: e}, instance\u003dinstance)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"class XenAPIBridgeDriver(XenVIFDriver):"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_beadb185","line":77,"in_reply_to":"da6ed579_8fd47020","updated":"2016-01-14 12:01:24.000000000","message":"Agreed.  Log the exception, then raise a Nova exception similar to 55.","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"14255ca75a9874d36cdf0e166ba21ac18e3ea779","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"},{"line_number":75,"context_line":"        except Exception as e:"},{"line_number":76,"context_line":"            LOG.debug(\"Fail to unplug vif %(vif)s, exception:%(exception)s\","},{"line_number":77,"context_line":"                      {\u0027vif\u0027: vif, \u0027exception\u0027: e}, instance\u003dinstance)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"class XenAPIBridgeDriver(XenVIFDriver):"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_cfc20353","line":77,"in_reply_to":"da6ed579_8fd47020","updated":"2016-01-15 02:57:08.000000000","message":"agreed, just one concern on the exception, see the reply below","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"14255ca75a9874d36cdf0e166ba21ac18e3ea779","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"},{"line_number":75,"context_line":"        except Exception as e:"},{"line_number":76,"context_line":"            LOG.debug(\"Fail to unplug vif %(vif)s, exception:%(exception)s\","},{"line_number":77,"context_line":"                      {\u0027vif\u0027: vif, \u0027exception\u0027: e}, instance\u003dinstance)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"class XenAPIBridgeDriver(XenVIFDriver):"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_efd10724","line":77,"in_reply_to":"da6ed579_beadb185","updated":"2016-01-15 02:57:08.000000000","message":"yes, can log the exception and raise Nova exception. But I have one concern on resize scenario: after resize, we will do confirm_migration \"https://github.com/openstack/nova/blob/5d6e0086b578077e7479b78e282e3f7da7ffca71/nova/virt/xenapi/vmops.py#L209\" which will unplug the original VIFs, in this case, if we raise the exception but the caller function didn\u0027t catch the exception, the resize will be regareded as failed?","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"efb37e13058e54f3acf3ab9089631dab48005e95","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"},{"line_number":75,"context_line":"        except Exception as e:"},{"line_number":76,"context_line":"            LOG.debug(\"Fail to unplug vif %(vif)s, exception:%(exception)s\","},{"line_number":77,"context_line":"                      {\u0027vif\u0027: vif, \u0027exception\u0027: e}, instance\u003dinstance)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"class XenAPIBridgeDriver(XenVIFDriver):"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_2225e907","line":77,"in_reply_to":"da6ed579_efd10724","updated":"2016-01-15 10:01:21.000000000","message":"I think that raising an exception here, and potentially failing the migration is acceptable.","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"d17e4108797315bd4300868efc97139d68e00273","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                _LW(\"Fail to unplug vif:%(vif)s, exception:%(exception)s\"),"},{"line_number":83,"context_line":"                {\u0027vif\u0027: vif, \u0027exception\u0027: e}, instance\u003dinstance)"},{"line_number":84,"context_line":"            raise exception.NovaException("},{"line_number":85,"context_line":"                reason\u003d_(\"Failed to unplug vif %s\") % vif)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"class XenAPIBridgeDriver(XenVIFDriver):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a68dd71_d70e6c3a","line":85,"updated":"2016-01-22 11:48:16.000000000","message":"John pointed out that we need a unit test for this exception being raised please.\n\nThanks Huan!","commit_id":"e51b47964705a86d29d4583281eeda7b0dbd18a9"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"4f0ad803cc7d3d90d9bcf5ad0876d10eca1d2830","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                _LW(\"Fail to unplug vif:%(vif)s, exception:%(exception)s\"),"},{"line_number":83,"context_line":"                {\u0027vif\u0027: vif, \u0027exception\u0027: e}, instance\u003dinstance)"},{"line_number":84,"context_line":"            raise exception.NovaException("},{"line_number":85,"context_line":"                reason\u003d_(\"Failed to unplug vif %s\") % vif)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"class XenAPIBridgeDriver(XenVIFDriver):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a68dd71_d39b9c16","line":85,"in_reply_to":"9a68dd71_d70e6c3a","updated":"2016-01-25 02:47:51.000000000","message":"ah, yes, sorry for missing adding new test, I will do this, thanks :)","commit_id":"e51b47964705a86d29d4583281eeda7b0dbd18a9"}],"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3d67ca5256d1199c75aa5c3aeb577de08668612c","unresolved":false,"context_lines":[{"line_number":1894,"context_line":"        for device, vif in enumerate(network_info):"},{"line_number":1895,"context_line":"            self.vif_driver.plug(instance, vif, device\u003ddevice)"},{"line_number":1896,"context_line":""},{"line_number":1897,"context_line":"    def unplug_vifs(self, instance, network_info, vm_ref\u003dNone):"},{"line_number":1898,"context_line":"        if network_info:"},{"line_number":1899,"context_line":"            for vif in network_info:"},{"line_number":1900,"context_line":"                self.vif_driver.unplug(instance, vif, vm_ref)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_221a406f","line":1897,"updated":"2016-01-14 11:40:14.000000000","message":"Nit: It does look a bit odd with vm_ref being optional like this. It feels like we should get the vm_ref here, rather than in the vif_driver.","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"14255ca75a9874d36cdf0e166ba21ac18e3ea779","unresolved":false,"context_lines":[{"line_number":1894,"context_line":"        for device, vif in enumerate(network_info):"},{"line_number":1895,"context_line":"            self.vif_driver.plug(instance, vif, device\u003ddevice)"},{"line_number":1896,"context_line":""},{"line_number":1897,"context_line":"    def unplug_vifs(self, instance, network_info, vm_ref\u003dNone):"},{"line_number":1898,"context_line":"        if network_info:"},{"line_number":1899,"context_line":"            for vif in network_info:"},{"line_number":1900,"context_line":"                self.vif_driver.unplug(instance, vif, vm_ref)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_af9eaf60","line":1897,"in_reply_to":"da6ed579_221a406f","updated":"2016-01-15 02:57:08.000000000","message":"yes, Done","commit_id":"9a608c70c50563cd85f03ecfb558af38cbd345df"}]}