)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     jianghuaw \u003cjianghua.wang@citrix.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-11-09 06:45:28 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"xenapi: OVS agent updates the wrong port when using Xen + Neutron"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"  With xen hypervisor, it will create two sets of port/interface for"},{"line_number":10,"context_line":"each VM vif: One is named as tapx.0 which is qemu-emulated NIC and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da85f559_901120af","line":7,"updated":"2015-11-09 15:28:28.000000000","message":"when using XenServer + Neutron","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     jianghuaw \u003cjianghua.wang@citrix.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-11-09 06:45:28 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"xenapi: OVS agent updates the wrong port when using Xen + Neutron"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"  With xen hypervisor, it will create two sets of port/interface for"},{"line_number":10,"context_line":"each VM vif: One is named as tapx.0 which is qemu-emulated NIC and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da85f559_1a78eac3","line":7,"in_reply_to":"da85f559_901120af","updated":"2015-11-09 16:00:09.000000000","message":"agree.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":7,"context_line":"xenapi: OVS agent updates the wrong port when using Xen + Neutron"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"  With xen hypervisor, it will create two sets of port/interface for"},{"line_number":10,"context_line":"each VM vif: One is named as tapx.0 which is qemu-emulated NIC and"},{"line_number":11,"context_line":"is used when no PV drivers installed with the VM; The other one is"},{"line_number":12,"context_line":"named as vifx.0 which is the xen network frontend NIC and is used"},{"line_number":13,"context_line":"when VM has PV drivers installed. But neutron only handle one port."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da85f559_d0cc8837","line":10,"updated":"2015-11-09 15:28:28.000000000","message":"actually \"tapX.Y\"; X \u003d domid, y\u003dVIF number in the domain.  The first NIC in domain 12 will be tap12.0 and vif12.0 and the third will be tap12.2 and vif12.2.\nJust change to \"tapX.Y\" - my expanded explanation isn\u0027t needed in the commit message.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":7,"context_line":"xenapi: OVS agent updates the wrong port when using Xen + Neutron"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"  With xen hypervisor, it will create two sets of port/interface for"},{"line_number":10,"context_line":"each VM vif: One is named as tapx.0 which is qemu-emulated NIC and"},{"line_number":11,"context_line":"is used when no PV drivers installed with the VM; The other one is"},{"line_number":12,"context_line":"named as vifx.0 which is the xen network frontend NIC and is used"},{"line_number":13,"context_line":"when VM has PV drivers installed. But neutron only handle one port."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da85f559_7a49f68a","line":10,"in_reply_to":"da85f559_d0cc8837","updated":"2015-11-09 16:00:09.000000000","message":"agree and will change in the next patch set","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":14,"context_line":"If the finally used port is not the one handled by neutron, it will"},{"line_number":15,"context_line":"have network issue."},{"line_number":16,"context_line":"  The solution is to change nova xenapi to add an interim bridge"},{"line_number":17,"context_line":"between the VM and the integration bridge which is similar as what\u0027s"},{"line_number":18,"context_line":"implemented in libvirt. In this way it will only expose to neutron"},{"line_number":19,"context_line":"with the integration bridge\u0027s port which connected to the interim"},{"line_number":20,"context_line":"bridge. So it will fix the issue mentioned above."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da85f559_10e3f0a4","line":17,"updated":"2015-11-09 15:28:28.000000000","message":"\"and the OVS integration bridge\"","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":14,"context_line":"If the finally used port is not the one handled by neutron, it will"},{"line_number":15,"context_line":"have network issue."},{"line_number":16,"context_line":"  The solution is to change nova xenapi to add an interim bridge"},{"line_number":17,"context_line":"between the VM and the integration bridge which is similar as what\u0027s"},{"line_number":18,"context_line":"implemented in libvirt. In this way it will only expose to neutron"},{"line_number":19,"context_line":"with the integration bridge\u0027s port which connected to the interim"},{"line_number":20,"context_line":"bridge. So it will fix the issue mentioned above."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da85f559_ba367e01","line":17,"in_reply_to":"da85f559_10e3f0a4","updated":"2015-11-09 16:00:09.000000000","message":"ok","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":17,"context_line":"between the VM and the integration bridge which is similar as what\u0027s"},{"line_number":18,"context_line":"implemented in libvirt. In this way it will only expose to neutron"},{"line_number":19,"context_line":"with the integration bridge\u0027s port which connected to the interim"},{"line_number":20,"context_line":"bridge. So it will fix the issue mentioned above."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Closes-Bug: #1268955"},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da85f559_508738b5","line":20,"updated":"2015-11-09 15:28:28.000000000","message":"+libvirt uses the interim bridge to apply iptables rules; so a side effect of adding this bridge is that we can apply security groups in the same way as libvirt.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":17,"context_line":"between the VM and the integration bridge which is similar as what\u0027s"},{"line_number":18,"context_line":"implemented in libvirt. In this way it will only expose to neutron"},{"line_number":19,"context_line":"with the integration bridge\u0027s port which connected to the interim"},{"line_number":20,"context_line":"bridge. So it will fix the issue mentioned above."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Closes-Bug: #1268955"},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da85f559_9d9b74c9","line":20,"in_reply_to":"da85f559_508738b5","updated":"2015-11-09 16:00:09.000000000","message":"exactly. But I think we may use another patch set to cover the security groups part basing on this implement.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Jianghua Wang \u003cjianghua.wang@citrix.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-02-24 09:33:36 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"xenapi: OVS agent updates the wrong port when using XenServer + Neutron"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"  With xen hypervisor, it will create two sets of port/interface for each VM vif: One is named as tapx.y which is qemu-emulated NIC and is used when no PV drivers installed with the VM; The other one is named as vifx.0 which is"},{"line_number":10,"context_line":"the xen network frontend NIC and is used when VM has PV drivers installed. But neutron only handle one port. If the finally used port is not the one handled by neutron, it will have network issue."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"9aed3d3a_2023b157","line":7,"updated":"2016-02-29 15:25:07.000000000","message":"Line too long (50 chars max) - see https://wiki.openstack.org/wiki/GitCommitMessages","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"xenapi: OVS agent updates the wrong port when using XenServer + Neutron"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"  With xen hypervisor, it will create two sets of port/interface for each VM vif: One is named as tapx.y which is qemu-emulated NIC and is used when no PV drivers installed with the VM; The other one is named as vifx.0 which is"},{"line_number":10,"context_line":"the xen network frontend NIC and is used when VM has PV drivers installed. But neutron only handle one port. If the finally used port is not the one handled by neutron, it will have network issue."},{"line_number":11,"context_line":"  The solution is to change nova xenapi to add an interim bridge between the VM and the OVS integration bridge. In this way it will only expose to neutron with the integration bridge\u0027s port which connected to the interim bridge."},{"line_number":12,"context_line":"So it will fix the issue mentioned above."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"9aed3d3a_c05be5f4","line":9,"updated":"2016-02-29 15:25:07.000000000","message":"Lines are too long; they need to wrap at 72 characters.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"58b6603d4bf6becbbe0066ef6510bb4bf2c65cac","unresolved":false,"context_lines":[{"line_number":12,"context_line":"other one is named as vifx.0 which is the xen network frontend"},{"line_number":13,"context_line":"NIC and is used when VM has PV drivers installed. But neutron"},{"line_number":14,"context_line":"only handle one port. If the finally used port is not the one"},{"line_number":15,"context_line":"handled by neutron, it will have network issue."},{"line_number":16,"context_line":"  The solution is to change nova xenapi to add an interim"},{"line_number":17,"context_line":"bridge between the VM and the OVS integration bridge. In this"},{"line_number":18,"context_line":"way it will only expose to neutron with the integration bridge\u0027s"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":34,"id":"fa0719c6_5d7288e6","line":15,"range":{"start_line":15,"start_character":20,"end_line":15,"end_character":46},"updated":"2016-03-21 18:18:46.000000000","message":"suggestion: \"the guest will have no network access\"","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"5ab30e8dc38c24303c283c29e64f9e46d0c0cae7","unresolved":false,"context_lines":[{"line_number":12,"context_line":"other one is named as vifx.0 which is the xen network frontend"},{"line_number":13,"context_line":"NIC and is used when VM has PV drivers installed. But neutron"},{"line_number":14,"context_line":"only handle one port. If the finally used port is not the one"},{"line_number":15,"context_line":"handled by neutron, it will have network issue."},{"line_number":16,"context_line":"  The solution is to change nova xenapi to add an interim"},{"line_number":17,"context_line":"bridge between the VM and the OVS integration bridge. In this"},{"line_number":18,"context_line":"way it will only expose to neutron with the integration bridge\u0027s"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":34,"id":"fa0719c6_1eda6d4c","line":15,"range":{"start_line":15,"start_character":20,"end_line":15,"end_character":46},"in_reply_to":"fa0719c6_5d7288e6","updated":"2016-03-23 07:43:51.000000000","message":"Done","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"58b6603d4bf6becbbe0066ef6510bb4bf2c65cac","unresolved":false,"context_lines":[{"line_number":13,"context_line":"NIC and is used when VM has PV drivers installed. But neutron"},{"line_number":14,"context_line":"only handle one port. If the finally used port is not the one"},{"line_number":15,"context_line":"handled by neutron, it will have network issue."},{"line_number":16,"context_line":"  The solution is to change nova xenapi to add an interim"},{"line_number":17,"context_line":"bridge between the VM and the OVS integration bridge. In this"},{"line_number":18,"context_line":"way it will only expose to neutron with the integration bridge\u0027s"},{"line_number":19,"context_line":"port which connected to the interim bridge. So it will fix the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":34,"id":"fa0719c6_276859f5","line":16,"range":{"start_line":16,"start_character":50,"end_line":16,"end_character":57},"updated":"2016-03-21 18:18:46.000000000","message":"remove interim","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"5ab30e8dc38c24303c283c29e64f9e46d0c0cae7","unresolved":false,"context_lines":[{"line_number":13,"context_line":"NIC and is used when VM has PV drivers installed. But neutron"},{"line_number":14,"context_line":"only handle one port. If the finally used port is not the one"},{"line_number":15,"context_line":"handled by neutron, it will have network issue."},{"line_number":16,"context_line":"  The solution is to change nova xenapi to add an interim"},{"line_number":17,"context_line":"bridge between the VM and the OVS integration bridge. In this"},{"line_number":18,"context_line":"way it will only expose to neutron with the integration bridge\u0027s"},{"line_number":19,"context_line":"port which connected to the interim bridge. So it will fix the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":34,"id":"fa0719c6_7edba94f","line":16,"range":{"start_line":16,"start_character":50,"end_line":16,"end_character":57},"in_reply_to":"fa0719c6_276859f5","updated":"2016-03-23 07:43:51.000000000","message":"Done","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"58b6603d4bf6becbbe0066ef6510bb4bf2c65cac","unresolved":false,"context_lines":[{"line_number":14,"context_line":"only handle one port. If the finally used port is not the one"},{"line_number":15,"context_line":"handled by neutron, it will have network issue."},{"line_number":16,"context_line":"  The solution is to change nova xenapi to add an interim"},{"line_number":17,"context_line":"bridge between the VM and the OVS integration bridge. In this"},{"line_number":18,"context_line":"way it will only expose to neutron with the integration bridge\u0027s"},{"line_number":19,"context_line":"port which connected to the interim bridge. So it will fix the"},{"line_number":20,"context_line":"issue mentioned above."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":34,"id":"fa0719c6_0759d55a","line":17,"range":{"start_line":17,"start_character":52,"end_line":17,"end_character":53},"updated":"2016-03-21 18:18:46.000000000","message":"suggestion: add (which we will call the \u0027interim bridge\u0027)","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"5ab30e8dc38c24303c283c29e64f9e46d0c0cae7","unresolved":false,"context_lines":[{"line_number":14,"context_line":"only handle one port. If the finally used port is not the one"},{"line_number":15,"context_line":"handled by neutron, it will have network issue."},{"line_number":16,"context_line":"  The solution is to change nova xenapi to add an interim"},{"line_number":17,"context_line":"bridge between the VM and the OVS integration bridge. In this"},{"line_number":18,"context_line":"way it will only expose to neutron with the integration bridge\u0027s"},{"line_number":19,"context_line":"port which connected to the interim bridge. So it will fix the"},{"line_number":20,"context_line":"issue mentioned above."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":34,"id":"fa0719c6_5ee065fa","line":17,"range":{"start_line":17,"start_character":52,"end_line":17,"end_character":53},"in_reply_to":"fa0719c6_0759d55a","updated":"2016-03-23 07:43:51.000000000","message":"Done","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"828f5a98cf0d7e42046707a96933ef609df7f63e","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"XenAPI: OVS agent updates the wrong port with Neutron"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"  With xen hypervisor, it will create two sets of port/interface"},{"line_number":10,"context_line":"for each VM vif: One is named as tapx.y which is qemu-emulated"},{"line_number":11,"context_line":"NIC and is used when no PV drivers installed with the VM; The"},{"line_number":12,"context_line":"other one is named as vifx.0 which is the xen network frontend"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":40,"id":"9abb7d3a_e5955087","line":9,"updated":"2016-05-31 14:03:07.000000000","message":"Paragraphs should not be indented in commit messages.","commit_id":"71fbc1e64662a26e022a916fa3878cc25e32efe4"}],"nova/tests/unit/virt/xenapi/stubs.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"610a697a0f33b631ff8517262a00c6e0cc7631b1","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    def fake_create_vifs(*args):"},{"line_number":34,"context_line":"        return [\u0027fake_vif_ref\u0027]"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    stubs.Set(vmops, \u0027_create_vifs\u0027, fake_create_vifs)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"def stubout_firewall_driver(stubs, conn):"}],"source_content_type":"text/x-python","patch_set":29,"id":"5aef4532_1c497ed0","line":36,"updated":"2016-03-10 15:15:51.000000000","message":"I think this is mox powered, we are generally dropping this and using mock instead.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"6f6e1dbd90f3389a3f00b3eac2b5fa0309376d98","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    def fake_create_vifs(*args):"},{"line_number":34,"context_line":"        return [\u0027fake_vif_ref\u0027]"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    stubs.Set(vmops, \u0027_create_vifs\u0027, fake_create_vifs)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"def stubout_firewall_driver(stubs, conn):"}],"source_content_type":"text/x-python","patch_set":29,"id":"1af94dfe_da5fbb70","line":36,"in_reply_to":"5aef4532_1c497ed0","updated":"2016-03-17 09:06:49.000000000","message":"Done","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"}],"nova/tests/unit/virt/xenapi/test_vif.py":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from mock import call"},{"line_number":17,"context_line":"from mock import Mock"},{"line_number":18,"context_line":"from mock import patch"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_6075b930","line":16,"updated":"2016-02-29 15:25:07.000000000","message":"Why change the import approach?  It\u0027s causing some unrelated changes in this file and would normally be part of a separate refactor change if it\u0027s needed.\n\nIt seems that no other files use the \"from mock import \u003cobject\u003e\" so more justification is needed.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from mock import call"},{"line_number":17,"context_line":"from mock import Mock"},{"line_number":18,"context_line":"from mock import patch"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_edda2716","line":16,"in_reply_to":"9aed3d3a_6075b930","updated":"2016-03-04 11:13:28.000000000","message":"Done","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"610a697a0f33b631ff8517262a00c6e0cc7631b1","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from mock import call"},{"line_number":17,"context_line":"from mock import Mock"},{"line_number":18,"context_line":"from mock import patch"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from nova import exception"},{"line_number":21,"context_line":"from nova.network import model"}],"source_content_type":"text/x-python","patch_set":29,"id":"5aef4532_66e909b7","line":18,"updated":"2016-03-10 15:15:51.000000000","message":"FYI:\nhttp://docs.openstack.org/developer/hacking/#imports","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"6f6e1dbd90f3389a3f00b3eac2b5fa0309376d98","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from mock import call"},{"line_number":17,"context_line":"from mock import Mock"},{"line_number":18,"context_line":"from mock import patch"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from nova import exception"},{"line_number":21,"context_line":"from nova.network import model"}],"source_content_type":"text/x-python","patch_set":29,"id":"3afc51ec_0f32395f","line":18,"in_reply_to":"5aef4532_66e909b7","updated":"2016-03-17 09:06:49.000000000","message":"Thanks. It\u0027s useful.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    return \"Unexpected call_xenapi: %s.%s\" % (method, args)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def fake_call_plugin(plugin, method, *args):"},{"line_number":75,"context_line":"    return"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_e0426967","line":74,"updated":"2016-02-29 15:25:07.000000000","message":"I think we need a more realistic return value here - a plugin shouldn\u0027t return None.  Perhaps this needs to be more like the fake_call_xenapi method above so an exception is eventually raised if an unexpected plugin is called?","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"610a697a0f33b631ff8517262a00c6e0cc7631b1","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    return \"Unexpected call_xenapi: %s.%s\" % (method, args)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def fake_call_plugin(plugin, method, *args):"},{"line_number":75,"context_line":"    return"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"5aef4532_c6fbbd77","line":74,"in_reply_to":"7af24918_4794bd89","updated":"2016-03-10 15:15:51.000000000","message":"don\u0027t forget you can just use mock for a lot of these.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    return \"Unexpected call_xenapi: %s.%s\" % (method, args)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def fake_call_plugin(plugin, method, *args):"},{"line_number":75,"context_line":"    return"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7af24918_4794bd89","line":74,"in_reply_to":"9aed3d3a_e0426967","updated":"2016-03-04 11:13:28.000000000","message":"Done","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        vm_ref \u003d \"fake_vm_ref\""},{"line_number":172,"context_line":"        self.bridge_driver.unplug(instance, fake_vif, vm_ref)"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        expected \u003d [call(\u0027VIF.destroy\u0027, \u0027fake_vif_ref\u0027)]"},{"line_number":175,"context_line":"        self.assertEqual(expected, self._session.call_xenapi.call_args_list)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_602e59a6","line":174,"updated":"2016-02-29 15:25:07.000000000","message":"There really is too much change in this file; we need to keep the imports as they were when adding new functionality / tests.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":236,"context_line":"        instance \u003d {\u0027name\u0027: \u0027fake_instance_name\u0027}"},{"line_number":237,"context_line":"        self.ovs_driver.post_start_actions(instance, vif_ref)"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"        add_p1 \u003d \u0027[\"ovs-vsctl\", \"--\", \u0027 \\"},{"line_number":240,"context_line":"                 \u0027\"--if-exists\", \"del-port\", \"fake_port_name1\", \"--\", \u0027 \\"},{"line_number":241,"context_line":"                 \u0027\"add-port\", \"fake_bridge\", \"fake_port_name1\", \"--\", \u0027 \\"},{"line_number":242,"context_line":"                 \u0027\"set\", \"interface\", \"fake_port_name1\", \u0027 \\"}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_43ffdfa9","line":239,"updated":"2016-02-29 15:25:07.000000000","message":"This feels very fragile; expecting exact matches for long OVS commands.\n\nCan we parse the calls and check that we have \u0027roughly\u0027 the right calls?\ne.g. look at _session.mock_calls; verify that [0] is the VIF.get_record, [1] is the network.get_bridge that [2] is a plugin call to network_config which includes add-port in the cmd, [3] includes add-port xapi1 and [4] includes external-ids:iface-id\u003dfake-nicira-iface-id?\n\nThoughts?","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"610a697a0f33b631ff8517262a00c6e0cc7631b1","unresolved":false,"context_lines":[{"line_number":236,"context_line":"        instance \u003d {\u0027name\u0027: \u0027fake_instance_name\u0027}"},{"line_number":237,"context_line":"        self.ovs_driver.post_start_actions(instance, vif_ref)"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"        add_p1 \u003d \u0027[\"ovs-vsctl\", \"--\", \u0027 \\"},{"line_number":240,"context_line":"                 \u0027\"--if-exists\", \"del-port\", \"fake_port_name1\", \"--\", \u0027 \\"},{"line_number":241,"context_line":"                 \u0027\"add-port\", \"fake_bridge\", \"fake_port_name1\", \"--\", \u0027 \\"},{"line_number":242,"context_line":"                 \u0027\"set\", \"interface\", \"fake_port_name1\", \u0027 \\"}],"source_content_type":"text/x-python","patch_set":29,"id":"5aef4532_267f01e1","line":239,"in_reply_to":"9aed3d3a_43ffdfa9","updated":"2016-03-10 15:15:51.000000000","message":"I think you need to split up the testing of this into smaller chunks.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"58b6603d4bf6becbbe0066ef6510bb4bf2c65cac","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    \u0027network_id\u0027: 123,"},{"line_number":33,"context_line":"    \u0027instance_uuid\u0027: \u0027fake-instance-uuid\u0027,"},{"line_number":34,"context_line":"    \u0027uuid\u0027: \u0027fake-uuid\u0027,"},{"line_number":35,"context_line":"}"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"class XenVIFDriverTestBase(stubs.XenAPITestBaseNoDB):"}],"source_content_type":"text/x-python","patch_set":34,"id":"fa0719c6_e7265e67","line":35,"updated":"2016-03-21 18:18:46.000000000","message":"Removing fake_call_xenapi feels like a refactor change rather than one needed for this functional change?\nIf we need to remove it so we can write the new tests, can this be split out into a refactor change?","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"5ab30e8dc38c24303c283c29e64f9e46d0c0cae7","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    \u0027network_id\u0027: 123,"},{"line_number":33,"context_line":"    \u0027instance_uuid\u0027: \u0027fake-instance-uuid\u0027,"},{"line_number":34,"context_line":"    \u0027uuid\u0027: \u0027fake-uuid\u0027,"},{"line_number":35,"context_line":"}"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"class XenVIFDriverTestBase(stubs.XenAPITestBaseNoDB):"}],"source_content_type":"text/x-python","patch_set":34,"id":"fa0719c6_3ebc91db","line":35,"in_reply_to":"fa0719c6_e7265e67","updated":"2016-03-23 07:43:51.000000000","message":"Done","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"58b6603d4bf6becbbe0066ef6510bb4bf2c65cac","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        super(XenVIFDriverTestBase, self).setUp()"},{"line_number":41,"context_line":"        self._session \u003d mock.Mock()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def mock_patch_object(self, target, attribute, return_val\u003dNone,"},{"line_number":44,"context_line":"                          side_effect\u003dNone):"},{"line_number":45,"context_line":"        # utilility function to mock runtime object\u0027s attribute"},{"line_number":46,"context_line":"        patcher \u003d mock.patch.object(target, attribute,"}],"source_content_type":"text/x-python","patch_set":34,"id":"fa0719c6_625cff25","line":43,"updated":"2016-03-21 18:18:46.000000000","message":"Why use this rather than @mock.patch_object() decorator on the test case?\nI think @mock.patch_object does this, but in a cleaner way? (no need for a custom cleanup)","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"5ab30e8dc38c24303c283c29e64f9e46d0c0cae7","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        super(XenVIFDriverTestBase, self).setUp()"},{"line_number":41,"context_line":"        self._session \u003d mock.Mock()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def mock_patch_object(self, target, attribute, return_val\u003dNone,"},{"line_number":44,"context_line":"                          side_effect\u003dNone):"},{"line_number":45,"context_line":"        # utilility function to mock runtime object\u0027s attribute"},{"line_number":46,"context_line":"        patcher \u003d mock.patch.object(target, attribute,"}],"source_content_type":"text/x-python","patch_set":34,"id":"fa0719c6_bea541a3","line":43,"in_reply_to":"fa0719c6_625cff25","updated":"2016-03-23 07:43:51.000000000","message":"This is specially needed for those objects which is inherited from\n XenAPISessionObject. I added some comments in the latest patch set.","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"7e4f54190e7ef3eb8e1397de07c7a782bc9e0333","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        self._session \u003d mock.Mock()"},{"line_number":68,"context_line":"        self._session.call_xenapi.side_effect \u003d fake_call_xenapi"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def mock_patch_object(self, target, attribute, return_val\u003dNone,"},{"line_number":71,"context_line":"                          side_effect\u003dNone):"},{"line_number":72,"context_line":"        \"\"\"Utilility function to mock object\u0027s attribute:"},{"line_number":73,"context_line":"        This is needed for those objects which is inherited from"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa0719c6_4d9c270f","line":70,"updated":"2016-03-23 13:07:55.000000000","message":"This should probably be somewhere more generic as we will have to use it more often; maybe in stubs.py?","commit_id":"1362a71c024564a21c2b6eac62c86a2665458df6"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7f05e717d7486973e057242a91120ab42cff4519","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        self._session \u003d mock.Mock()"},{"line_number":68,"context_line":"        self._session.call_xenapi.side_effect \u003d fake_call_xenapi"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def mock_patch_object(self, target, attribute, return_val\u003dNone,"},{"line_number":71,"context_line":"                          side_effect\u003dNone):"},{"line_number":72,"context_line":"        \"\"\"Utilility function to mock object\u0027s attribute:"},{"line_number":73,"context_line":"        This is needed for those objects which is inherited from"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa0719c6_c0b2844b","line":70,"in_reply_to":"fa0719c6_4d9c270f","updated":"2016-03-24 10:20:50.000000000","message":"Presently we will keep it in this test class and we can find a proper place to hold it if other testing need use this function in the future. thanks.","commit_id":"1362a71c024564a21c2b6eac62c86a2665458df6"}],"nova/tests/unit/virt/xenapi/test_vmops.py":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"7e4f54190e7ef3eb8e1397de07c7a782bc9e0333","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                                          \"0/0000:00:00.0\")"},{"line_number":409,"context_line":"        else:"},{"line_number":410,"context_line":"            pci_manager.get_instance_pci_devs(instance).AndReturn([])"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"        step +\u003d 1"},{"line_number":413,"context_line":"        self.vmops._update_instance_progress(context, instance, step, steps)"},{"line_number":414,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"fa0719c6_8d38ff3e","line":411,"updated":"2016-03-23 13:07:55.000000000","message":"Remove new line","commit_id":"1362a71c024564a21c2b6eac62c86a2665458df6"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7f05e717d7486973e057242a91120ab42cff4519","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                                          \"0/0000:00:00.0\")"},{"line_number":409,"context_line":"        else:"},{"line_number":410,"context_line":"            pci_manager.get_instance_pci_devs(instance).AndReturn([])"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"        step +\u003d 1"},{"line_number":413,"context_line":"        self.vmops._update_instance_progress(context, instance, step, steps)"},{"line_number":414,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"fa0719c6_a0add0ea","line":411,"in_reply_to":"fa0719c6_8d38ff3e","updated":"2016-03-24 10:20:50.000000000","message":"Done","commit_id":"1362a71c024564a21c2b6eac62c86a2665458df6"}],"nova/virt/xenapi/vif.py":[{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"64a4135f3b50c62e9cdbda09477ae5a966b5a075","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    def __init__(self, xenapi_session):"},{"line_number":42,"context_line":"        self._session \u003d xenapi_session"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"class XenAPIBridgeDriver(XenVIFDriver):"},{"line_number":46,"context_line":"    \"\"\"VIF Driver for XenAPI that uses XenAPI to create Networks.\"\"\""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_561186ab","line":44,"updated":"2015-11-09 08:05:27.000000000","message":"need similar change in this class; But not understand why this class is needed at here? does it support other bridges besides OVS at here? Linux bridge?","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    def __init__(self, xenapi_session):"},{"line_number":42,"context_line":"        self._session \u003d xenapi_session"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"class XenAPIBridgeDriver(XenVIFDriver):"},{"line_number":46,"context_line":"    \"\"\"VIF Driver for XenAPI that uses XenAPI to create Networks.\"\"\""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_7aaa56d6","line":44,"in_reply_to":"da85f559_561186ab","updated":"2015-11-09 15:28:28.000000000","message":"Just need to make sure the API is the same.  Don\u0027t need to make changes to the XenAPIBridgeDriver class.  It\u0027s the default, but can probably be deprecated when we move to Neutron with the OVS?","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        if network is None:"},{"line_number":178,"context_line":"            return"},{"line_number":179,"context_line":"        bridge_name \u003d self._session.call_xenapi(\u0027network.get_bridge\u0027, network)"},{"line_number":180,"context_line":"        v1_name, v2_name \u003d self.get_veth_pair_names(vif[\u0027id\u0027])"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        LOG.debug(\u0027VMnetwork is %(network)s, bridge is %(br)s\u0027,"},{"line_number":183,"context_line":"                  {\u0027network\u0027: network, \u0027br\u0027: bridge_name})"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_e7fc6f40","line":180,"updated":"2015-11-09 15:28:28.000000000","message":"I don\u0027t understand what v1_name and v2_name are; they should have more descriptive variable names","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        if network is None:"},{"line_number":178,"context_line":"            return"},{"line_number":179,"context_line":"        bridge_name \u003d self._session.call_xenapi(\u0027network.get_bridge\u0027, network)"},{"line_number":180,"context_line":"        v1_name, v2_name \u003d self.get_veth_pair_names(vif[\u0027id\u0027])"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        LOG.debug(\u0027VMnetwork is %(network)s, bridge is %(br)s\u0027,"},{"line_number":183,"context_line":"                  {\u0027network\u0027: network, \u0027br\u0027: bridge_name})"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_5dcaeca2","line":180,"in_reply_to":"da85f559_e7fc6f40","updated":"2015-11-09 16:00:09.000000000","message":"yes, I will rename them in the next patch set.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        self.map_external_ids_with_vif(v2_name, vif_rec)"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def get_vif_interim_net_name(self, vif):"},{"line_number":213,"context_line":"        return (\"qnet\" + vif[\u0027id\u0027])[:network_model.NIC_NAME_LEN]"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def create_vif_interim_network(self, vif):"},{"line_number":216,"context_line":"        net_name \u003d self.get_vif_interim_net_name(vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_c7684bf8","line":213,"updated":"2015-11-09 15:28:28.000000000","message":"why qnet?  That\u0027s very Neutron specific and implies that the code should be in Neutron instead of Nova.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                              {\u0027vif_id\u0027: vif[\u0027id\u0027]})"},{"line_number":225,"context_line":"        return network_ref"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def get_veth_pair_names(self, iface_id):"},{"line_number":228,"context_line":"        return ((\"qvb%s\" % iface_id)[:network_model.NIC_NAME_LEN],"},{"line_number":229,"context_line":"                (\"qvo%s\" % iface_id)[:network_model.NIC_NAME_LEN])"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_0775f3f7","line":227,"updated":"2015-11-09 15:28:28.000000000","message":"why qvb and qvo? I don\u0027t know what the b and o stand for.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                              {\u0027vif_id\u0027: vif[\u0027id\u0027]})"},{"line_number":225,"context_line":"        return network_ref"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def get_veth_pair_names(self, iface_id):"},{"line_number":228,"context_line":"        return ((\"qvb%s\" % iface_id)[:network_model.NIC_NAME_LEN],"},{"line_number":229,"context_line":"                (\"qvo%s\" % iface_id)[:network_model.NIC_NAME_LEN])"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_9da134ac","line":227,"in_reply_to":"da85f559_0775f3f7","updated":"2015-11-09 16:00:09.000000000","message":"just referred to the naming way from libvirt and please see the chart of \"Network traffic flow - north/south\" from the following link. \nhttp://docs.openstack.org/networking-guide/scenario_provider_ovs.html","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        return ((\"qvb%s\" % iface_id)[:network_model.NIC_NAME_LEN],"},{"line_number":229,"context_line":"                (\"qvo%s\" % iface_id)[:network_model.NIC_NAME_LEN])"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def add_patch_port(self, bridge_name, port_name, peer_port_name):"},{"line_number":232,"context_line":"        cmd_args \u003d [\u0027ovs-vsctl\u0027, \u0027--\u0027, \u0027--if-exists\u0027, \u0027del-port\u0027, port_name,"},{"line_number":233,"context_line":"                    \u0027--\u0027, \u0027add-port\u0027, bridge_name, port_name,"},{"line_number":234,"context_line":"                    \u0027--\u0027, \u0027set\u0027, \u0027interface\u0027, port_name,"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_3a384ed3","line":231,"updated":"2015-11-09 15:28:28.000000000","message":"Internal functions should start with _","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        return ((\"qvb%s\" % iface_id)[:network_model.NIC_NAME_LEN],"},{"line_number":229,"context_line":"                (\"qvo%s\" % iface_id)[:network_model.NIC_NAME_LEN])"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def add_patch_port(self, bridge_name, port_name, peer_port_name):"},{"line_number":232,"context_line":"        cmd_args \u003d [\u0027ovs-vsctl\u0027, \u0027--\u0027, \u0027--if-exists\u0027, \u0027del-port\u0027, port_name,"},{"line_number":233,"context_line":"                    \u0027--\u0027, \u0027add-port\u0027, bridge_name, port_name,"},{"line_number":234,"context_line":"                    \u0027--\u0027, \u0027set\u0027, \u0027interface\u0027, port_name,"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_fd73a04b","line":231,"in_reply_to":"da85f559_3a384ed3","updated":"2015-11-09 16:00:09.000000000","message":"got it and will change it. thanks.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":258,"context_line":"                   ]"},{"line_number":259,"context_line":"        self.exec_plugin_cmd(cmd_args)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def exec_plugin_cmd(self, cmd_args):"},{"line_number":262,"context_line":"        args \u003d {"},{"line_number":263,"context_line":"            \u0027cmd\u0027: jsonutils.dumps(cmd_args),"},{"line_number":264,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_878803e9","line":261,"updated":"2015-11-09 15:28:28.000000000","message":"suggest \"_exec_dom0\" rather than exec_plugin_cmd as what we\u0027re trying to do is run this in dom0.  That this happens to be implemented through a plugin is not relevant here.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":258,"context_line":"                   ]"},{"line_number":259,"context_line":"        self.exec_plugin_cmd(cmd_args)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def exec_plugin_cmd(self, cmd_args):"},{"line_number":262,"context_line":"        args \u003d {"},{"line_number":263,"context_line":"            \u0027cmd\u0027: jsonutils.dumps(cmd_args),"},{"line_number":264,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_7d9ed057","line":261,"in_reply_to":"da85f559_878803e9","updated":"2015-11-09 16:00:09.000000000","message":"good point. thanks.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"af5f6db84053add4e522ce11b0bbbb2bb6c7ecd5","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        else:"},{"line_number":71,"context_line":"            vif_rec[\u0027qos_algorithm_type\u0027] \u003d \u0027\u0027"},{"line_number":72,"context_line":"            vif_rec[\u0027qos_algorithm_params\u0027] \u003d {}"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        return vif_rec"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def _ensure_vlan_bridge(self, network):"}],"source_content_type":"text/x-python","patch_set":7,"id":"da85f559_f176f6d5","line":73,"updated":"2015-11-11 16:42:16.000000000","message":"Can you remove this line?  Not relevant to the change.","commit_id":"4795794e1fa23e19f28aec1e5c33ed6f716f453f"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"89b41ee323a39ef67c53ab953d258c218b393b2e","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        else:"},{"line_number":71,"context_line":"            vif_rec[\u0027qos_algorithm_type\u0027] \u003d \u0027\u0027"},{"line_number":72,"context_line":"            vif_rec[\u0027qos_algorithm_params\u0027] \u003d {}"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        return vif_rec"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def _ensure_vlan_bridge(self, network):"}],"source_content_type":"text/x-python","patch_set":7,"id":"da85f559_7f7f2dab","line":73,"in_reply_to":"da85f559_f176f6d5","updated":"2015-11-11 17:15:25.000000000","message":"sure.","commit_id":"4795794e1fa23e19f28aec1e5c33ed6f716f453f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"af5f6db84053add4e522ce11b0bbbb2bb6c7ecd5","unresolved":false,"context_lines":[{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def post_start_actions(self, instance, vif_ref):"},{"line_number":194,"context_line":"        \"\"\"Do needed actions post vif start:"},{"line_number":195,"context_line":"        plug the interim ovs bridge to the integration bridge"},{"line_number":196,"context_line":"        \"\"\""},{"line_number":197,"context_line":"        vif_rec \u003d self._session.call_xenapi(\u0027VIF.get_record\u0027, vif_ref)"},{"line_number":198,"context_line":"        network_ref \u003d vif_rec[\u0027network\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"da85f559_34230ca9","line":195,"updated":"2015-11-11 16:42:16.000000000","message":"Also note that we need to copy the external IDs from the VIF we\u0027ve just created to the port the OVS will see?","commit_id":"4795794e1fa23e19f28aec1e5c33ed6f716f453f"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"89b41ee323a39ef67c53ab953d258c218b393b2e","unresolved":false,"context_lines":[{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def post_start_actions(self, instance, vif_ref):"},{"line_number":194,"context_line":"        \"\"\"Do needed actions post vif start:"},{"line_number":195,"context_line":"        plug the interim ovs bridge to the integration bridge"},{"line_number":196,"context_line":"        \"\"\""},{"line_number":197,"context_line":"        vif_rec \u003d self._session.call_xenapi(\u0027VIF.get_record\u0027, vif_ref)"},{"line_number":198,"context_line":"        network_ref \u003d vif_rec[\u0027network\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"da85f559_1aa86fda","line":195,"in_reply_to":"da85f559_34230ca9","updated":"2015-11-11 17:15:25.000000000","message":"yes. will add note for that. it\u0027s done within _map_external_ids_with_vif(patch_port2, vif_rec),\nbut it\u0027s not a copy but directly populate the needed external id. Do you prefer to copy all the external IDs from the new created VIF?","commit_id":"4795794e1fa23e19f28aec1e5c33ed6f716f453f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"af5f6db84053add4e522ce11b0bbbb2bb6c7ecd5","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        return network_ref"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _get_patch_port_pair_names(self, iface_id):"},{"line_number":234,"context_line":"        return ((\"vifp1-%s\" % iface_id)[:network_model.NIC_NAME_LEN],"},{"line_number":235,"context_line":"                (\"vifp2-%s\" % iface_id)[:network_model.NIC_NAME_LEN])"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def _add_patch_port(self, bridge_name, port_name, peer_port_name):"}],"source_content_type":"text/x-python","patch_set":7,"id":"da85f559_14a36801","line":234,"updated":"2015-11-11 16:42:16.000000000","message":"Sorry but I think we should stick with qvo/qvb as you pointed out in the other change.  I don\u0027t want new conventions and those familiar with how the libvirt system works will be familiar with this one.","commit_id":"4795794e1fa23e19f28aec1e5c33ed6f716f453f"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"89b41ee323a39ef67c53ab953d258c218b393b2e","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        return network_ref"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _get_patch_port_pair_names(self, iface_id):"},{"line_number":234,"context_line":"        return ((\"vifp1-%s\" % iface_id)[:network_model.NIC_NAME_LEN],"},{"line_number":235,"context_line":"                (\"vifp2-%s\" % iface_id)[:network_model.NIC_NAME_LEN])"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def _add_patch_port(self, bridge_name, port_name, peer_port_name):"}],"source_content_type":"text/x-python","patch_set":7,"id":"da85f559_1a5b8ff7","line":234,"in_reply_to":"da85f559_14a36801","updated":"2015-11-11 17:15:25.000000000","message":"I tended to do that firstly. But changed my mind later as our this interim bridge in fact is not at the same location as the linux bridge libvirt. Imaging that how to name the ports when we need to add the additional Linux bridge to enable security group?","commit_id":"4795794e1fa23e19f28aec1e5c33ed6f716f453f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"8624277e2e405fd1d432e11d3e4251040a02e4b6","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        return network_ref"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _get_patch_port_pair_names(self, iface_id):"},{"line_number":234,"context_line":"        return ((\"vifp1-%s\" % iface_id)[:network_model.NIC_NAME_LEN],"},{"line_number":235,"context_line":"                (\"vifp2-%s\" % iface_id)[:network_model.NIC_NAME_LEN])"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def _add_patch_port(self, bridge_name, port_name, peer_port_name):"}],"source_content_type":"text/x-python","patch_set":7,"id":"da85f559_45c93825","line":234,"in_reply_to":"da85f559_1a5b8ff7","updated":"2015-11-11 17:36:09.000000000","message":"Can\u0027t we use the bridge for the same purpose as well?\nBut for us it would serve two purposes - to hold security groups and to join the vif+tap ports together?","commit_id":"4795794e1fa23e19f28aec1e5c33ed6f716f453f"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"894dae6561be4cff900819bc06ebe384d689229d","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        return network_ref"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _get_patch_port_pair_names(self, iface_id):"},{"line_number":234,"context_line":"        return ((\"vifp1-%s\" % iface_id)[:network_model.NIC_NAME_LEN],"},{"line_number":235,"context_line":"                (\"vifp2-%s\" % iface_id)[:network_model.NIC_NAME_LEN])"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def _add_patch_port(self, bridge_name, port_name, peer_port_name):"}],"source_content_type":"text/x-python","patch_set":7,"id":"da85f559_3810c856","line":234,"in_reply_to":"da85f559_45c93825","updated":"2015-11-12 01:52:14.000000000","message":"It seems OVS is not compatible with iptable rules basing on the following link:\nhttp://openvswitch.org/pipermail/discuss/2013-November/011990.html\nAnd that seems to be the reason why libvirt added the interim linux bridge. So I\u0027m thinking that we need the chain to look as this: VM vif -\u003e OVS iterim bridge -\u003e Linux bridge -\u003e int-br. \nNote: If we can plug the VM vif to Linux bridge directly, we will have same chain as libvirt; but XenServer XAPI by default supports OVS and we can\u0027t change it to Linux bridge directly.","commit_id":"4795794e1fa23e19f28aec1e5c33ed6f716f453f"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"efc97bcd774880757d1acb7ea0db0c69f583b4e8","unresolved":false,"context_lines":[{"line_number":206,"context_line":"                  {\u0027port1\u0027: patch_port1, \u0027port2\u0027: patch_port2,"},{"line_number":207,"context_line":"                   \u0027network_ref\u0027: network_ref,"},{"line_number":208,"context_line":"                   \u0027bridge_name\u0027: bridge_name})"},{"line_number":209,"context_line":"        if bridge_name is None:"},{"line_number":210,"context_line":"            raise Exception(_(\"Can\u0027t find bridge for vif_ref:%(vif_ref)s of\""},{"line_number":211,"context_line":"                              \"VM:%(vm_name)s\"),"},{"line_number":212,"context_line":"                            {\u0027vif_ref\u0027: vif_ref, \u0027vm_name\u0027: instance[\u0027name\u0027]})"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_7f9b5173","line":209,"updated":"2015-11-13 02:42:36.000000000","message":"just a tiny suggestion, this bridge_name if condition check can be done immediately after getting the bridge_name(line:199-200)","commit_id":"35e515a698ea8ae27ea86e64ab233c365cf93e0a"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"13565b96cc95bd4fc907a7b110fd62bb98c2a24f","unresolved":false,"context_lines":[{"line_number":206,"context_line":"                  {\u0027port1\u0027: patch_port1, \u0027port2\u0027: patch_port2,"},{"line_number":207,"context_line":"                   \u0027network_ref\u0027: network_ref,"},{"line_number":208,"context_line":"                   \u0027bridge_name\u0027: bridge_name})"},{"line_number":209,"context_line":"        if bridge_name is None:"},{"line_number":210,"context_line":"            raise Exception(_(\"Can\u0027t find bridge for vif_ref:%(vif_ref)s of\""},{"line_number":211,"context_line":"                              \"VM:%(vm_name)s\"),"},{"line_number":212,"context_line":"                            {\u0027vif_ref\u0027: vif_ref, \u0027vm_name\u0027: instance[\u0027name\u0027]})"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_8547c44a","line":209,"in_reply_to":"da85f559_7f9b5173","updated":"2015-11-13 03:50:35.000000000","message":"the purpose is to print debug info before raise exception.","commit_id":"35e515a698ea8ae27ea86e64ab233c365cf93e0a"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"7091633a536daa34cc14c7d6112f49e1a8a65774","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        LOG.debug(\u0027VMnetwork is %(network)s, bridge is %(br)s\u0027,"},{"line_number":186,"context_line":"                  {\u0027network\u0027: network, \u0027br\u0027: bridge_name})"},{"line_number":187,"context_line":"        self._del_ovs_br(bridge_name)"},{"line_number":188,"context_line":"        self._session.call_xenapi(\u0027network.destroy\u0027, network)"},{"line_number":189,"context_line":"        # delete the port from integration bridge"},{"line_number":190,"context_line":"        self._del_ovs_port(CONF.xenserver.ovs_integration_bridge, patch_port2)"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba8a016a_895df816","line":187,"updated":"2015-11-19 09:55:56.000000000","message":"XAPI will delete the OVS bridge when we call network.destroy.","commit_id":"6a3fd42525e9db1690d2ba5a15adc532321e11f9"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"6175ad0f3d98be4b427acd99d0a2943e5e396178","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        LOG.debug(\u0027VMnetwork is %(network)s, bridge is %(br)s\u0027,"},{"line_number":186,"context_line":"                  {\u0027network\u0027: network, \u0027br\u0027: bridge_name})"},{"line_number":187,"context_line":"        self._del_ovs_br(bridge_name)"},{"line_number":188,"context_line":"        self._session.call_xenapi(\u0027network.destroy\u0027, network)"},{"line_number":189,"context_line":"        # delete the port from integration bridge"},{"line_number":190,"context_line":"        self._del_ovs_port(CONF.xenserver.ovs_integration_bridge, patch_port2)"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba8a016a_942f2f3e","line":187,"in_reply_to":"ba8a016a_895df816","updated":"2015-11-19 16:56:58.000000000","message":"But it seems the ovs br won\u0027t be deleted automatically with the xapi network destroy. So maybe we have to use ovs-vsctl or I missing anything?","commit_id":"6a3fd42525e9db1690d2ba5a15adc532321e11f9"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"7091633a536daa34cc14c7d6112f49e1a8a65774","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        LOG.debug(\u0027VMnetwork is %(network)s, bridge is %(br)s\u0027,"},{"line_number":186,"context_line":"                  {\u0027network\u0027: network, \u0027br\u0027: bridge_name})"},{"line_number":187,"context_line":"        self._del_ovs_br(bridge_name)"},{"line_number":188,"context_line":"        self._session.call_xenapi(\u0027network.destroy\u0027, network)"},{"line_number":189,"context_line":"        # delete the port from integration bridge"},{"line_number":190,"context_line":"        self._del_ovs_port(CONF.xenserver.ovs_integration_bridge, patch_port2)"},{"line_number":191,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"ba8a016a_09cd88b6","line":188,"updated":"2015-11-19 09:55:56.000000000","message":"Can we do this when the VIF still exists?\nDo we need to delete the VIF from the VM first, or has this already happened?","commit_id":"6a3fd42525e9db1690d2ba5a15adc532321e11f9"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"6175ad0f3d98be4b427acd99d0a2943e5e396178","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        LOG.debug(\u0027VMnetwork is %(network)s, bridge is %(br)s\u0027,"},{"line_number":186,"context_line":"                  {\u0027network\u0027: network, \u0027br\u0027: bridge_name})"},{"line_number":187,"context_line":"        self._del_ovs_br(bridge_name)"},{"line_number":188,"context_line":"        self._session.call_xenapi(\u0027network.destroy\u0027, network)"},{"line_number":189,"context_line":"        # delete the port from integration bridge"},{"line_number":190,"context_line":"        self._del_ovs_port(CONF.xenserver.ovs_integration_bridge, patch_port2)"},{"line_number":191,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"ba8a016a_b40b53d5","line":188,"in_reply_to":"ba8a016a_09cd88b6","updated":"2015-11-19 16:56:58.000000000","message":"vif has gone with VM destroy which happens before this point.","commit_id":"6a3fd42525e9db1690d2ba5a15adc532321e11f9"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"7091633a536daa34cc14c7d6112f49e1a8a65774","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        self._exec_dom0_cmd(cmd_args)"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def _del_ovs_br(self, bridge_name):"},{"line_number":251,"context_line":"        cmd_args \u003d [\u0027ovs-vsctl\u0027, \u0027--\u0027, \u0027--if-exists\u0027, \u0027del-br\u0027, bridge_name]"},{"line_number":252,"context_line":"        self._exec_dom0_cmd(cmd_args)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def _map_external_ids_with_vif(self, interface, vif_rec):"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba8a016a_ae600202","line":251,"updated":"2015-11-19 09:55:56.000000000","message":"Shouldn\u0027t do this; if XAPI creates the bridge, XAPI must delete it.","commit_id":"6a3fd42525e9db1690d2ba5a15adc532321e11f9"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"6175ad0f3d98be4b427acd99d0a2943e5e396178","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        self._exec_dom0_cmd(cmd_args)"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def _del_ovs_br(self, bridge_name):"},{"line_number":251,"context_line":"        cmd_args \u003d [\u0027ovs-vsctl\u0027, \u0027--\u0027, \u0027--if-exists\u0027, \u0027del-br\u0027, bridge_name]"},{"line_number":252,"context_line":"        self._exec_dom0_cmd(cmd_args)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def _map_external_ids_with_vif(self, interface, vif_rec):"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba8a016a_f4b8fb12","line":251,"in_reply_to":"ba8a016a_ae600202","updated":"2015-11-19 16:56:58.000000000","message":"But it seems the ovs br won\u0027t be deleted automatically with the xapi network destroy. So maybe we have to use ovs-vsctl or I missing anything?","commit_id":"6a3fd42525e9db1690d2ba5a15adc532321e11f9"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"2b2d7357ca64837787a6745ec541bd69d843b450","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        LOG.debug(\u0027VMnetwork is %(network)s, bridge is %(br)s\u0027,"},{"line_number":186,"context_line":"                  {\u0027network\u0027: network, \u0027br\u0027: bridge_name})"},{"line_number":187,"context_line":"        self._del_ovs_br(bridge_name)"},{"line_number":188,"context_line":"        self._session.call_xenapi(\u0027network.destroy\u0027, network)"},{"line_number":189,"context_line":"        # delete the port from integration bridge"},{"line_number":190,"context_line":"        self._del_ovs_port(CONF.xenserver.ovs_integration_bridge, patch_port2)"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba8a016a_7664e8e4","line":187,"updated":"2015-11-20 10:50:19.000000000","message":"In my test the OVS bridge was deleted as soon as the VM using it shut down:\n\n[root@zarss ~]# xe network-list  params\u003dname-label,bridge name-label\u003dtest\nname-label ( RW)    : test\n        bridge ( RO): xapi8\n[root@zarss ~]# ovs-vsctl show | grep Bridge\n...\n    Bridge \"xapi8\"\n[root@zarss ~]# xe vm-shutdown force\u003dtrue vm\u003dOther\\ install\\ media\\ \\(1\\)\n[root@zarss ~]# ovs-vsctl show | grep Bridge\n\u003cno xapi8\u003e\n\nCertainly you shouldn\u0027t delete the ovs_br before calling XAPI to destroy e network.","commit_id":"a5e7415b8f6a133254967cfb1ba1dc2c32a83cdd"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"0be84ca708567234e59cce98c3aae5c8a13434d0","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        LOG.debug(\u0027VMnetwork is %(network)s, bridge is %(br)s\u0027,"},{"line_number":186,"context_line":"                  {\u0027network\u0027: network, \u0027br\u0027: bridge_name})"},{"line_number":187,"context_line":"        self._del_ovs_br(bridge_name)"},{"line_number":188,"context_line":"        self._session.call_xenapi(\u0027network.destroy\u0027, network)"},{"line_number":189,"context_line":"        # delete the port from integration bridge"},{"line_number":190,"context_line":"        self._del_ovs_port(CONF.xenserver.ovs_integration_bridge, patch_port2)"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba8a016a_66e18f9a","line":187,"in_reply_to":"ba8a016a_458fa8cc","updated":"2015-11-20 14:22:30.000000000","message":"Bob: Sorry but more thinking...\nI think the logic is as this: VM vif destroyed(VM shut down will trigger it) -\u003e check if the vif is the only one connected with the OVS? -\u003e if yes, then it destroys the ovs br; otherwise it keeps it. this action won\u0027t be done again with network destroy.\n  As we have the patch ported on the OVS br, the bridge is not destroyed.\n  So if we don\u0027t delete OVS separately at here, we have to split this function to two parts: 1st - remove the patch ports; 2nd destroy the network. the 1st part should go before hard_shutdown_vm(); and the 2nd part goes after destroy_vm(). Could you confirm if we should go with that?\n  And I thought of another special case: if someone shutdown the VM from somewhere out of nova e.g. cmdline from the VM instance or from the hypervisor. And then delete this instance from nova. As the VM is down already, it won\u0027t call xapi to shutdown VM. Perhaps it\u0027s not safe to depend on the shutdown xapi. \n  So it seem not a bad idea to invoke ovs-vsctl to destroy this OVS? Thanks.","commit_id":"a5e7415b8f6a133254967cfb1ba1dc2c32a83cdd"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"641c40cd5feeae99efe190bb88cda69ea53eb99d","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        LOG.debug(\u0027VMnetwork is %(network)s, bridge is %(br)s\u0027,"},{"line_number":186,"context_line":"                  {\u0027network\u0027: network, \u0027br\u0027: bridge_name})"},{"line_number":187,"context_line":"        self._del_ovs_br(bridge_name)"},{"line_number":188,"context_line":"        self._session.call_xenapi(\u0027network.destroy\u0027, network)"},{"line_number":189,"context_line":"        # delete the port from integration bridge"},{"line_number":190,"context_line":"        self._del_ovs_port(CONF.xenserver.ovs_integration_bridge, patch_port2)"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba8a016a_dc5b6af5","line":187,"in_reply_to":"ba8a016a_66e18f9a","updated":"2015-11-20 14:48:06.000000000","message":"Understood; please just add a bit of explanation.\nI think we should probably delete the port first though, if we can?\n\ni.e. the order should be:\ndelete port\ndestroy network\n# Comment to explain why XAPI did not delete the bridge\ndelete ovs bridge if it still exists.","commit_id":"a5e7415b8f6a133254967cfb1ba1dc2c32a83cdd"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"412a524c2fc2cce222e991f49b60e99da6db6990","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        LOG.debug(\u0027VMnetwork is %(network)s, bridge is %(br)s\u0027,"},{"line_number":186,"context_line":"                  {\u0027network\u0027: network, \u0027br\u0027: bridge_name})"},{"line_number":187,"context_line":"        self._del_ovs_br(bridge_name)"},{"line_number":188,"context_line":"        self._session.call_xenapi(\u0027network.destroy\u0027, network)"},{"line_number":189,"context_line":"        # delete the port from integration bridge"},{"line_number":190,"context_line":"        self._del_ovs_port(CONF.xenserver.ovs_integration_bridge, patch_port2)"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba8a016a_458fa8cc","line":187,"in_reply_to":"ba8a016a_7664e8e4","updated":"2015-11-20 12:44:19.000000000","message":"Thanks for your carefulness. I did verified the code by not run ovs-vsctl to delete the ovs br and it didn\u0027t get deleted via network destroy. After getting this comment, I double re-thought of it and did more test. I think I understand the gap now. I added a patch port on the ovs br. We should manually remove that port firstly before destroy the network. otherwise ovs br will be kept due to the existing of patch port on it.\n So absolutely I will change the code. Thanks again.","commit_id":"a5e7415b8f6a133254967cfb1ba1dc2c32a83cdd"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"8543a73bf41e41b96f8cb8ea7d93f374b42e8b85","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        \"\"\""},{"line_number":233,"context_line":"        try:"},{"line_number":234,"context_line":"            super(XenAPIOpenVswitchDriver, self).unplug(instance, vif)"},{"line_number":235,"context_line":"            vm_ref \u003d vm_utils.lookup(self._session, instance[\u0027name\u0027])"},{"line_number":236,"context_line":"            if not vm_ref:"},{"line_number":237,"context_line":"                LOG.debug(\"vm didn\u0027t exist, no need to unplug vif %s\","},{"line_number":238,"context_line":"                        vif, instance\u003dinstance)"},{"line_number":239,"context_line":"                return"},{"line_number":240,"context_line":"            vif_ref \u003d self._get_vif_ref(vif, vm_ref)"},{"line_number":241,"context_line":"            if not vif_ref:"},{"line_number":242,"context_line":"                LOG.debug(\"vif didn\u0027t exist, no need to unplug vif %s\","},{"line_number":243,"context_line":"                      vif, instance\u003dinstance)"},{"line_number":244,"context_line":"                return"},{"line_number":245,"context_line":"            self._session.call_xenapi(\u0027VIF.destroy\u0027, vif_ref)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"            net_name \u003d self.get_vif_interim_net_name(vif)"},{"line_number":248,"context_line":"            network \u003d network_utils.find_network_with_name_label("}],"source_content_type":"text/x-python","patch_set":22,"id":"1a7b0d38_d7b2cbe4","line":245,"range":{"start_line":235,"start_character":12,"end_line":245,"end_character":61},"updated":"2015-12-29 09:18:52.000000000","message":"remove this part, they are in super","commit_id":"8aa6e6108fb97dbcbb1b71b9abcb4b6de5a2c7e7"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"c3c57d6ef6e99cb705f317f987057a278eb82777","unresolved":false,"context_lines":[{"line_number":234,"context_line":"            super(XenAPIOpenVswitchDriver, self).unplug(instance, vif)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"            net_name \u003d self.get_vif_interim_net_name(vif)"},{"line_number":237,"context_line":"            network \u003d network_utils.find_network_with_name_label("},{"line_number":238,"context_line":"                self._session, net_name)"},{"line_number":239,"context_line":"            if network is None:"},{"line_number":240,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa69d971_d265031e","line":237,"updated":"2016-01-05 11:49:43.000000000","message":"we can\u0027t depend on the net name to find the network due to the net name is identify for VM resizing.","commit_id":"ada59b64c39d06a7fc9debb50ae81e49361b2d25"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            raise exception.NovaException("},{"line_number":64,"context_line":"                reason\u003d_(\"Failed to create vif %s\") % vif)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        LOG.debug(\"create vif %(vif)s for vm %(vm_ref)s successfully\","},{"line_number":67,"context_line":"                  {\u0027vif\u0027: vif, \u0027vm_ref\u0027: vm_ref})"},{"line_number":68,"context_line":"        return vif_ref"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_a3a34bb1","side":"PARENT","line":66,"updated":"2016-02-29 15:25:07.000000000","message":"Why was this log line removed?  Does it not make sense in the new approach, or is it logged elsewhere?","commit_id":"7847f75eb293c0d31f5c5a0147aa15d5c631136c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            raise exception.NovaException("},{"line_number":64,"context_line":"                reason\u003d_(\"Failed to create vif %s\") % vif)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        LOG.debug(\"create vif %(vif)s for vm %(vm_ref)s successfully\","},{"line_number":67,"context_line":"                  {\u0027vif\u0027: vif, \u0027vm_ref\u0027: vm_ref})"},{"line_number":68,"context_line":"        return vif_ref"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7af24918_7ead0de8","side":"PARENT","line":66,"in_reply_to":"9aed3d3a_a3a34bb1","updated":"2016-03-04 11:13:28.000000000","message":"This file has been changed much by several changes. At the very beginning when Huan and I were changing this file, there was no debug in this vif.py but all of the needed log were done by the upper calling function. So I deleted it and add the log into the function of _create_vifs() which calling it. But now the things have been changed a lot by recent patch sets. I prefer to keep this debug log at here.","commit_id":"7847f75eb293c0d31f5c5a0147aa15d5c631136c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":213,"context_line":"            device \u003d 0"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"        # create an interim network which has a bridge directly connected"},{"line_number":216,"context_line":"        # to instance"},{"line_number":217,"context_line":"        network_ref \u003d self.create_vif_interim_network(vif)"},{"line_number":218,"context_line":"        vif_rec \u003d {}"},{"line_number":219,"context_line":"        vif_rec[\u0027device\u0027] \u003d str(device)"}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_8b045918","line":216,"updated":"2016-02-29 15:25:07.000000000","message":"Suggestion: Create an interim network for each VIF, so dom0 has a single bridge for each device (the emulated and PV ethernet devices will both be on this bridge)","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":213,"context_line":"            device \u003d 0"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"        # create an interim network which has a bridge directly connected"},{"line_number":216,"context_line":"        # to instance"},{"line_number":217,"context_line":"        network_ref \u003d self.create_vif_interim_network(vif)"},{"line_number":218,"context_line":"        vif_rec \u003d {}"},{"line_number":219,"context_line":"        vif_rec[\u0027device\u0027] \u003d str(device)"}],"source_content_type":"text/x-python","patch_set":29,"id":"7af24918_3ef545ec","line":216,"in_reply_to":"9aed3d3a_8b045918","updated":"2016-03-04 11:13:28.000000000","message":"Done","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":244,"context_line":"                self._session, net_name)"},{"line_number":245,"context_line":"            if network is None:"},{"line_number":246,"context_line":"                return"},{"line_number":247,"context_line":"            vifs \u003d self._session.call_xenapi(\u0027network.get_VIFs\u0027, network)"},{"line_number":248,"context_line":"            if vifs:"},{"line_number":249,"context_line":"                # only remove the interim network when it\u0027s empty."},{"line_number":250,"context_line":"                # for resize/migrate on local host, vifs on both of the"}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_eecc13bd","line":247,"updated":"2016-02-29 15:25:07.000000000","message":"Can you test if the \"new\" way of calling functions works here?  I think it should, and if it does I\u0027d like all of the call_xenapi methods to change:\n\nvifs\u003dself._session.network.get_VIFs(network)\n\nI keep forgetting this, but we do want to try and move towards the new invocation model (it\u0027s required if we want to do any real performance testing, so that calls aren\u0027t all hidden behind a call_xenapi function)","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            # delete the patch port pair"},{"line_number":261,"context_line":"            self._del_ovs_port(bridge_name, patch_port1)"},{"line_number":262,"context_line":"            self._del_ovs_port(CONF.xenserver.ovs_integration_bridge,"},{"line_number":263,"context_line":"                               patch_port2)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"            LOG.debug(\u0027destroying network: network\u003d%(network)s,\u0027"},{"line_number":266,"context_line":"                      \u0027bridge\u003d%(br)s\u0027,"}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_63dae318","line":263,"updated":"2016-02-29 15:25:07.000000000","message":"I think these lines (254-263) should always happen, even if there are other VIFs on the network?\nWe\u0027re destroying _this_ VIF so should always delete the patch port for this one?","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            # delete the patch port pair"},{"line_number":261,"context_line":"            self._del_ovs_port(bridge_name, patch_port1)"},{"line_number":262,"context_line":"            self._del_ovs_port(CONF.xenserver.ovs_integration_bridge,"},{"line_number":263,"context_line":"                               patch_port2)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"            LOG.debug(\u0027destroying network: network\u003d%(network)s,\u0027"},{"line_number":266,"context_line":"                      \u0027bridge\u003d%(br)s\u0027,"}],"source_content_type":"text/x-python","patch_set":29,"id":"7af24918_1977c3a0","line":263,"in_reply_to":"9aed3d3a_63dae318","updated":"2016-03-04 11:13:28.000000000","message":"See the comments, we can\u0027t remove the patch ports. as the remaining VIF depends on it to be connected to the integration br in the scenario of resizing/migration.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":273,"context_line":"            # at here."},{"line_number":274,"context_line":"            self._del_ovs_br(bridge_name)"},{"line_number":275,"context_line":"        except Exception as e:"},{"line_number":276,"context_line":"            LOG.debug(\"Fail to unplug vif %(vif)s, exception:%(exception)s\","},{"line_number":277,"context_line":"                      {\u0027vif\u0027: vif, \u0027exception\u0027: e}, instance\u003dinstance)"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"    def post_start_actions(self, instance, vif_ref):"}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_0e55f7f8","line":276,"updated":"2016-02-29 15:25:07.000000000","message":"I think this should raise a Nova exception rather than swallow it? c.f. XenVIFDriver.unplug","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":273,"context_line":"            # at here."},{"line_number":274,"context_line":"            self._del_ovs_br(bridge_name)"},{"line_number":275,"context_line":"        except Exception as e:"},{"line_number":276,"context_line":"            LOG.debug(\"Fail to unplug vif %(vif)s, exception:%(exception)s\","},{"line_number":277,"context_line":"                      {\u0027vif\u0027: vif, \u0027exception\u0027: e}, instance\u003dinstance)"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"    def post_start_actions(self, instance, vif_ref):"}],"source_content_type":"text/x-python","patch_set":29,"id":"7af24918_b9432fae","line":276,"in_reply_to":"9aed3d3a_0e55f7f8","updated":"2016-03-04 11:13:28.000000000","message":"Done","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        iface_id \u003d vif_rec[\u0027other_config\u0027][\u0027nicira-iface-id\u0027]"},{"line_number":290,"context_line":"        patch_port1, patch_port2 \u003d self._get_patch_port_pair_names(iface_id)"},{"line_number":291,"context_line":"        LOG.debug(\u0027plug_ovs_bridge: port1\u003d%(port1)s, port2\u003d%(port2)s,\u0027"},{"line_number":292,"context_line":"                  \u0027network_ref\u003d%(network_ref)s, bridge_name\u003d%(bridge_name)s\u0027,"},{"line_number":293,"context_line":"                  {\u0027port1\u0027: patch_port1, \u0027port2\u0027: patch_port2,"},{"line_number":294,"context_line":"                   \u0027network_ref\u0027: network_ref,"},{"line_number":295,"context_line":"                   \u0027bridge_name\u0027: bridge_name})"}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_2e955b86","line":292,"updated":"2016-02-29 15:25:07.000000000","message":"network_ref is an OpaqueRef which is useless outside of XAPI.  Please log the network UUID instead (self._session.call_xenapi(\u0027network.get_uuid\u0027, network_ref)","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        iface_id \u003d vif_rec[\u0027other_config\u0027][\u0027nicira-iface-id\u0027]"},{"line_number":290,"context_line":"        patch_port1, patch_port2 \u003d self._get_patch_port_pair_names(iface_id)"},{"line_number":291,"context_line":"        LOG.debug(\u0027plug_ovs_bridge: port1\u003d%(port1)s, port2\u003d%(port2)s,\u0027"},{"line_number":292,"context_line":"                  \u0027network_ref\u003d%(network_ref)s, bridge_name\u003d%(bridge_name)s\u0027,"},{"line_number":293,"context_line":"                  {\u0027port1\u0027: patch_port1, \u0027port2\u0027: patch_port2,"},{"line_number":294,"context_line":"                   \u0027network_ref\u0027: network_ref,"},{"line_number":295,"context_line":"                   \u0027bridge_name\u0027: bridge_name})"}],"source_content_type":"text/x-python","patch_set":29,"id":"7af24918_14d305d6","line":292,"in_reply_to":"9aed3d3a_2e955b86","updated":"2016-03-04 11:13:28.000000000","message":"Done","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            return network_ref"},{"line_number":321,"context_line":"        network_ref \u003d self._session.call_xenapi(\u0027network.create\u0027, network_rec)"},{"line_number":322,"context_line":"        if network_ref is None:"},{"line_number":323,"context_line":"            raise Exception(_(\"Failed to create the interim netowrk for vif -\""},{"line_number":324,"context_line":"                              \"%(vif_id)s\"),"},{"line_number":325,"context_line":"                              {\u0027vif_id\u0027: vif[\u0027id\u0027]})"},{"line_number":326,"context_line":"        return network_ref"}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_ee009368","line":323,"updated":"2016-02-29 15:25:07.000000000","message":"Typo: network","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            return network_ref"},{"line_number":321,"context_line":"        network_ref \u003d self._session.call_xenapi(\u0027network.create\u0027, network_rec)"},{"line_number":322,"context_line":"        if network_ref is None:"},{"line_number":323,"context_line":"            raise Exception(_(\"Failed to create the interim netowrk for vif -\""},{"line_number":324,"context_line":"                              \"%(vif_id)s\"),"},{"line_number":325,"context_line":"                              {\u0027vif_id\u0027: vif[\u0027id\u0027]})"},{"line_number":326,"context_line":"        return network_ref"}],"source_content_type":"text/x-python","patch_set":29,"id":"7af24918_9448b5b4","line":323,"in_reply_to":"9aed3d3a_ee009368","updated":"2016-03-04 11:13:28.000000000","message":"Done","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":359,"context_line":"                   ]"},{"line_number":360,"context_line":"        self._exec_dom0_cmd(cmd_args)"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"    def _exec_dom0_cmd(self, cmd_args):"},{"line_number":363,"context_line":"        args \u003d {"},{"line_number":364,"context_line":"            \u0027cmd\u0027: jsonutils.dumps(cmd_args),"},{"line_number":365,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_e39d530b","line":362,"updated":"2016-02-29 15:25:07.000000000","message":"Can we use call_plugin_serialized which should handle converting cmd_args into something that can go over the wire?\n\nWe may have to change the function definition and that may mean it could not be in xenhost (ATM you can\u0027t register plugins twice for one file).  If so then we can have a discussion.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":359,"context_line":"                   ]"},{"line_number":360,"context_line":"        self._exec_dom0_cmd(cmd_args)"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"    def _exec_dom0_cmd(self, cmd_args):"},{"line_number":363,"context_line":"        args \u003d {"},{"line_number":364,"context_line":"            \u0027cmd\u0027: jsonutils.dumps(cmd_args),"},{"line_number":365,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":29,"id":"7af24918_271ff15b","line":362,"in_reply_to":"9aed3d3a_e39d530b","updated":"2016-03-04 11:13:28.000000000","message":"Yes, I will change to use call_plugin_serialized; But I didn\u0027t understand why it couldn\u0027t be in xenhost if we have to change the function definition.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"58b6603d4bf6becbbe0066ef6510bb4bf2c65cac","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self._session \u003d xenapi_session"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def _get_vif_ref(self, vif, vm_ref):"},{"line_number":48,"context_line":"        vif_refs \u003d self._session.VM.get_VIFs(vm_ref)"},{"line_number":49,"context_line":"        for vif_ref in vif_refs:"},{"line_number":50,"context_line":"            try:"},{"line_number":51,"context_line":"                vif_rec \u003d self._session.VIF.get_record(vif_ref)"}],"source_content_type":"text/x-python","patch_set":34,"id":"fa0719c6_e252cfdc","line":48,"updated":"2016-03-21 18:18:46.000000000","message":"Sorry - I should have been clearer.  Only new calls should really change to use the self._session.VM.get_VIFs approach.\nIf we\u0027re going to change the other calls, we should do so in a separate refactor change.\n\nChanging _all_ of the calls in this file makes this change much bigger than it needs to be.","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"5ab30e8dc38c24303c283c29e64f9e46d0c0cae7","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self._session \u003d xenapi_session"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def _get_vif_ref(self, vif, vm_ref):"},{"line_number":48,"context_line":"        vif_refs \u003d self._session.VM.get_VIFs(vm_ref)"},{"line_number":49,"context_line":"        for vif_ref in vif_refs:"},{"line_number":50,"context_line":"            try:"},{"line_number":51,"context_line":"                vif_rec \u003d self._session.VIF.get_record(vif_ref)"}],"source_content_type":"text/x-python","patch_set":34,"id":"fa0719c6_3eb811c8","line":48,"in_reply_to":"fa0719c6_e252cfdc","updated":"2016-03-23 07:43:51.000000000","message":"Done","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"7e4f54190e7ef3eb8e1397de07c7a782bc9e0333","unresolved":false,"context_lines":[{"line_number":27,"context_line":"from nova.virt.xenapi import network_utils"},{"line_number":28,"context_line":"from nova.virt.xenapi import vm_utils"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"xenapi_ovs_integration_bridge_opt \u003d cfg.StrOpt(\u0027ovs_integration_bridge\u0027,"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa0719c6_0dd7cfed","line":30,"updated":"2016-03-23 13:07:55.000000000","message":"Duplicate line (see #40) - remove","commit_id":"1362a71c024564a21c2b6eac62c86a2665458df6"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7f05e717d7486973e057242a91120ab42cff4519","unresolved":false,"context_lines":[{"line_number":27,"context_line":"from nova.virt.xenapi import network_utils"},{"line_number":28,"context_line":"from nova.virt.xenapi import vm_utils"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"xenapi_ovs_integration_bridge_opt \u003d cfg.StrOpt(\u0027ovs_integration_bridge\u0027,"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa0719c6_8d88ca72","line":30,"in_reply_to":"fa0719c6_0dd7cfed","updated":"2016-03-24 10:20:50.000000000","message":"Done","commit_id":"1362a71c024564a21c2b6eac62c86a2665458df6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"828f5a98cf0d7e42046707a96933ef609df7f63e","unresolved":false,"context_lines":[{"line_number":237,"context_line":"        4. delete the OVS bridge service for the interim network"},{"line_number":238,"context_line":"        \"\"\""},{"line_number":239,"context_line":"        try:"},{"line_number":240,"context_line":"            super(XenAPIOpenVswitchDriver, self).unplug(instance, vif, vm_ref)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"            net_name \u003d self.get_vif_interim_net_name(vif)"},{"line_number":243,"context_line":"            network \u003d network_utils.find_network_with_name_label("}],"source_content_type":"text/x-python","patch_set":40,"id":"9abb7d3a_65a8408e","line":240,"updated":"2016-05-31 14:03:07.000000000","message":"Putting your super() call in a try: block is pretty unconventional. Do we really expect this to raise an exception?","commit_id":"71fbc1e64662a26e022a916fa3878cc25e32efe4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"828f5a98cf0d7e42046707a96933ef609df7f63e","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            # won\u0027t be destroyed automatically by XAPI. So let\u0027s destroy it"},{"line_number":272,"context_line":"            # at here."},{"line_number":273,"context_line":"            self._ovs_del_br(bridge_name)"},{"line_number":274,"context_line":"        except Exception as e:"},{"line_number":275,"context_line":"            LOG.warn(_LW(\"Fail to unplug vif %(if)s, exception:%(exception)s\"),"},{"line_number":276,"context_line":"                     {\u0027if\u0027: vif, \u0027exception\u0027: e}, instance\u003dinstance)"},{"line_number":277,"context_line":"            raise exception.NovaException("}],"source_content_type":"text/x-python","patch_set":40,"id":"9abb7d3a_a5e068fb","line":274,"updated":"2016-05-31 14:03:07.000000000","message":"Please come up with a better exception to catch here. Further, this is a huge try block. Ideally, smaller chunks of code should be inside a try block, with specific exceptions and handling of those specific error cases.","commit_id":"71fbc1e64662a26e022a916fa3878cc25e32efe4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"828f5a98cf0d7e42046707a96933ef609df7f63e","unresolved":false,"context_lines":[{"line_number":274,"context_line":"        except Exception as e:"},{"line_number":275,"context_line":"            LOG.warn(_LW(\"Fail to unplug vif %(if)s, exception:%(exception)s\"),"},{"line_number":276,"context_line":"                     {\u0027if\u0027: vif, \u0027exception\u0027: e}, instance\u003dinstance)"},{"line_number":277,"context_line":"            raise exception.NovaException("},{"line_number":278,"context_line":"                reason\u003d_(\"Failed to unplug vif %s\") % vif)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def post_start_actions(self, instance, vif_ref):"}],"source_content_type":"text/x-python","patch_set":40,"id":"9abb7d3a_c5800cfa","line":277,"updated":"2016-05-31 14:03:07.000000000","message":"Surely you can raise something more specific right? This is akin to \"raise Exception()\" which is not cool.","commit_id":"71fbc1e64662a26e022a916fa3878cc25e32efe4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"828f5a98cf0d7e42046707a96933ef609df7f63e","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            return network_ref"},{"line_number":321,"context_line":"        network_ref \u003d self._session.network.create(network_rec)"},{"line_number":322,"context_line":"        if network_ref is None:"},{"line_number":323,"context_line":"            raise Exception(_(\"Failed to create the interim network for vif -\""},{"line_number":324,"context_line":"                              \"%(vif_id)s\"),"},{"line_number":325,"context_line":"                              {\u0027vif_id\u0027: vif[\u0027id\u0027]})"},{"line_number":326,"context_line":"        return network_ref"}],"source_content_type":"text/x-python","patch_set":40,"id":"9abb7d3a_a59ce85d","line":323,"updated":"2016-05-31 14:03:07.000000000","message":"Please don\u0027t do this. Raise a specific exception.","commit_id":"71fbc1e64662a26e022a916fa3878cc25e32efe4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c63b9f61ae34321f41c34f812dbb7374ba4e7c5","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            return network_ref"},{"line_number":322,"context_line":"        try:"},{"line_number":323,"context_line":"            network_ref \u003d self._session.network.create(network_rec)"},{"line_number":324,"context_line":"        except Exception as e:"},{"line_number":325,"context_line":"            LOG.warn(_LW(\"Failed to create interim network for vif %(if)s, \""},{"line_number":326,"context_line":"                         \"exception:%(exception)s\"),"},{"line_number":327,"context_line":"                     {\u0027if\u0027: vif, \u0027exception\u0027: e})"}],"source_content_type":"text/x-python","patch_set":44,"id":"3aaa91ec_d2b4e4d2","line":324,"updated":"2016-06-21 16:49:08.000000000","message":"I still don\u0027t like catching this only, as it seems the paths in question should be super predictable. But maybe if we wouldn\u0027t do anything different it\u0027s not so bad.","commit_id":"97745f2bee288f1f88649f92e3c3e377d3e014fa"}],"nova/virt/xenapi/vm_utils.py":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":1685,"context_line":"    return vdi_refs"},{"line_number":1686,"context_line":""},{"line_number":1687,"context_line":""},{"line_number":1688,"context_line":"def lookup_vm_vifs(session, vm_ref):"},{"line_number":1689,"context_line":"    \"\"\"Look for the VIFs that are attached to the VM.\"\"\""},{"line_number":1690,"context_line":"    try:"},{"line_number":1691,"context_line":"        vif_refs \u003d session.call_xenapi(\"VM.get_VIFs\", vm_ref)"}],"source_content_type":"text/x-python","patch_set":29,"id":"9aed3d3a_6ea5a345","line":1688,"updated":"2016-02-29 15:25:07.000000000","message":"Why do we need this as a separate function?\n\nIf XAPI raises an exception the return vif_refs will be undefined and raise again.\nThere also isn\u0027t any real value to the function since it just logs the failure?","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":1685,"context_line":"    return vdi_refs"},{"line_number":1686,"context_line":""},{"line_number":1687,"context_line":""},{"line_number":1688,"context_line":"def lookup_vm_vifs(session, vm_ref):"},{"line_number":1689,"context_line":"    \"\"\"Look for the VIFs that are attached to the VM.\"\"\""},{"line_number":1690,"context_line":"    try:"},{"line_number":1691,"context_line":"        vif_refs \u003d session.call_xenapi(\"VM.get_VIFs\", vm_ref)"}],"source_content_type":"text/x-python","patch_set":29,"id":"7af24918_7457e957","line":1688,"in_reply_to":"9aed3d3a_6ea5a345","updated":"2016-03-04 11:13:28.000000000","message":"Done","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"610a697a0f33b631ff8517262a00c6e0cc7631b1","unresolved":false,"context_lines":[{"line_number":1691,"context_line":"        vif_refs \u003d session.call_xenapi(\"VM.get_VIFs\", vm_ref)"},{"line_number":1692,"context_line":"    except session.XenAPI.Failure:"},{"line_number":1693,"context_line":"        LOG.exception(_LE(\u0027\"Look for the VIFs failed\u0027))"},{"line_number":1694,"context_line":"    return vif_refs"},{"line_number":1695,"context_line":""},{"line_number":1696,"context_line":""},{"line_number":1697,"context_line":"def lookup(session, name_label, check_rescue\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":29,"id":"5aef4532_46668d2a","line":1694,"updated":"2016-03-10 15:15:51.000000000","message":"if you raise an exception vif_refs is not defined here. I think you meant to re-raise the above exception maybe?","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"}],"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":545,"context_line":"            self.inject_network_info(instance, network_info, vm_ref)"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"        @step"},{"line_number":548,"context_line":"        def create_vifs_step(undo_mgr, vm_ref):"},{"line_number":549,"context_line":"            return self._create_vifs(instance, vm_ref, network_info)"},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"        @step"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_30b15490","line":548,"updated":"2015-11-09 15:28:28.000000000","message":"I don\u0027t think we want to add another step; the number of steps is quite well defined; perhaps \"create_vifs\" should be in \u0027attach_devices\u0027?","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":1947,"context_line":"            vif_refs.append(vif_ref)"},{"line_number":1948,"context_line":"        return vif_refs"},{"line_number":1949,"context_line":""},{"line_number":1950,"context_line":"    def _plug_ovs_with_vif(self, instance, vif_refs):"},{"line_number":1951,"context_line":"        \"\"\"plug to the integration network\"\"\""},{"line_number":1952,"context_line":"        for vif_ref in vif_refs:"},{"line_number":1953,"context_line":"            self.vif_driver.plug_ovs_bridge(instance, vif_ref)"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_473f7bb4","line":1950,"updated":"2015-11-09 15:28:28.000000000","message":"This name should be more generic - perhaps _vif_post_start_actions","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":1947,"context_line":"            vif_refs.append(vif_ref)"},{"line_number":1948,"context_line":"        return vif_refs"},{"line_number":1949,"context_line":""},{"line_number":1950,"context_line":"    def _plug_ovs_with_vif(self, instance, vif_refs):"},{"line_number":1951,"context_line":"        \"\"\"plug to the integration network\"\"\""},{"line_number":1952,"context_line":"        for vif_ref in vif_refs:"},{"line_number":1953,"context_line":"            self.vif_driver.plug_ovs_bridge(instance, vif_ref)"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_ddfc9c40","line":1950,"in_reply_to":"da85f559_473f7bb4","updated":"2015-11-09 16:00:09.000000000","message":"agree.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":1950,"context_line":"    def _plug_ovs_with_vif(self, instance, vif_refs):"},{"line_number":1951,"context_line":"        \"\"\"plug to the integration network\"\"\""},{"line_number":1952,"context_line":"        for vif_ref in vif_refs:"},{"line_number":1953,"context_line":"            self.vif_driver.plug_ovs_bridge(instance, vif_ref)"},{"line_number":1954,"context_line":""},{"line_number":1955,"context_line":"    def plug_vifs(self, instance, network_info):"},{"line_number":1956,"context_line":"        \"\"\"Set up VIF networking on the host.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_c7a04bf7","line":1953,"updated":"2015-11-09 15:28:28.000000000","message":"Again, more generic: self.vif_driver.post_start_actions(instance, vif_ref)?","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":1950,"context_line":"    def _plug_ovs_with_vif(self, instance, vif_refs):"},{"line_number":1951,"context_line":"        \"\"\"plug to the integration network\"\"\""},{"line_number":1952,"context_line":"        for vif_ref in vif_refs:"},{"line_number":1953,"context_line":"            self.vif_driver.plug_ovs_bridge(instance, vif_ref)"},{"line_number":1954,"context_line":""},{"line_number":1955,"context_line":"    def plug_vifs(self, instance, network_info):"},{"line_number":1956,"context_line":"        \"\"\"Set up VIF networking on the host.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_1db1c48f","line":1953,"in_reply_to":"da85f559_c7a04bf7","updated":"2015-11-09 16:00:09.000000000","message":"agree.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"7091633a536daa34cc14c7d6112f49e1a8a65774","unresolved":false,"context_lines":[{"line_number":585,"context_line":"                    kernel_file, ramdisk_file)"},{"line_number":586,"context_line":"            vif_refs \u003d attach_devices_step(undo_mgr, instance, vm_ref, vdis,"},{"line_number":587,"context_line":"                                           disk_image_type, network_info)"},{"line_number":588,"context_line":"            LOG.debug(\u0027vif_refs is %s\u0027, vif_refs)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"            inject_instance_data_step(undo_mgr, vm_ref, vdis)"},{"line_number":591,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"ba8a016a_09290837","line":588,"updated":"2015-11-19 09:55:56.000000000","message":"LOG.debug(\u0027Attaching the following vif_refs: %s\u0027, vif_refs)","commit_id":"6a3fd42525e9db1690d2ba5a15adc532321e11f9"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"6175ad0f3d98be4b427acd99d0a2943e5e396178","unresolved":false,"context_lines":[{"line_number":585,"context_line":"                    kernel_file, ramdisk_file)"},{"line_number":586,"context_line":"            vif_refs \u003d attach_devices_step(undo_mgr, instance, vm_ref, vdis,"},{"line_number":587,"context_line":"                                           disk_image_type, network_info)"},{"line_number":588,"context_line":"            LOG.debug(\u0027vif_refs is %s\u0027, vif_refs)"},{"line_number":589,"context_line":""},{"line_number":590,"context_line":"            inject_instance_data_step(undo_mgr, vm_ref, vdis)"},{"line_number":591,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"ba8a016a_f4c23b52","line":588,"in_reply_to":"ba8a016a_09290837","updated":"2015-11-19 16:56:58.000000000","message":"yes, I will refine the log in the next patch-set. thanks.","commit_id":"6a3fd42525e9db1690d2ba5a15adc532321e11f9"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"f0d2f1b3c55c61fde582d9790abd894fae366e5e","unresolved":false,"context_lines":[{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        @step"},{"line_number":556,"context_line":"        def setup_network_step(undo_mgr, vm_ref, vif_refs):"},{"line_number":557,"context_line":"            self._vif_post_start_actions(instance, vif_refs)"},{"line_number":558,"context_line":"            self._prepare_instance_filter(instance, network_info)"},{"line_number":559,"context_line":""},{"line_number":560,"context_line":"        @step"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a8ffd7b_a0889ae8","line":557,"updated":"2015-11-25 03:53:42.000000000","message":"should add an undo function to roll back the change on the ports and bridge in case it failed to spawn VM.","commit_id":"bdc4f60397be3b77b9566b1ba0023ff37bef5450"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"3f224fd1f73f8fbc3d8c6f5f310dd5b5d09f92aa","unresolved":false,"context_lines":[{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        @step"},{"line_number":556,"context_line":"        def setup_network_step(undo_mgr, vm_ref, vif_refs):"},{"line_number":557,"context_line":"            self._vif_post_start_actions(instance, vif_refs)"},{"line_number":558,"context_line":"            self._prepare_instance_filter(instance, network_info)"},{"line_number":559,"context_line":""},{"line_number":560,"context_line":"        @step"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a8ffd7b_dfaa5019","line":557,"in_reply_to":"9a8ffd7b_a0889ae8","updated":"2015-11-25 09:31:22.000000000","message":"Ah, it\u0027s covered by undo_create_vm which will invoke vif.unplug to roll back the ports/bridge change. So there is no need to add more undo function.","commit_id":"bdc4f60397be3b77b9566b1ba0023ff37bef5450"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"27f228759d365d418c6c064e16d35eea25e4f4b0","unresolved":false,"context_lines":[{"line_number":349,"context_line":"            bad_volumes_callback(bad_devices)"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def _post_start_actions(self, instance, vif_refs, resize):"},{"line_number":352,"context_line":"        if resize:"},{"line_number":353,"context_line":"            return"},{"line_number":354,"context_line":"        for vif_ref in vif_refs:"},{"line_number":355,"context_line":"            self.vif_driver.post_start_actions(instance, vif_ref)"}],"source_content_type":"text/x-python","patch_set":25,"id":"da6ed579_91e420ac","line":352,"range":{"start_line":352,"start_character":11,"end_line":352,"end_character":17},"updated":"2016-01-15 11:41:06.000000000","message":"resize param here is not reliable, ref Line:441 \u0026 Line:449, it is always True now, so resize scenario is OK, but for normal scenario of booting an instance, it cannot work properly as the left of post_start_action will not be called any more","commit_id":"bafc3425615add09161189ea12134c71b4d9edb8"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"b50fe07b8c19017273bf677ac763ee3821b29f98","unresolved":false,"context_lines":[{"line_number":349,"context_line":"            bad_volumes_callback(bad_devices)"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def _post_start_actions(self, instance, vif_refs, resize):"},{"line_number":352,"context_line":"        if resize:"},{"line_number":353,"context_line":"            return"},{"line_number":354,"context_line":"        for vif_ref in vif_refs:"},{"line_number":355,"context_line":"            self.vif_driver.post_start_actions(instance, vif_ref)"}],"source_content_type":"text/x-python","patch_set":25,"id":"da6ed579_47d8d8ef","line":352,"range":{"start_line":352,"start_character":11,"end_line":352,"end_character":17},"in_reply_to":"da6ed579_91e420ac","updated":"2016-01-18 08:19:03.000000000","message":"thanks huan. Understand now that resize at here just means if disk need to be resized. So we can\u0027t depend on this flag to determine if it should invoke post_start_actions on vif. I think we can go with another solution: force boot on paused regardless boot_on is true or not; so that the interim bridge will display; then force shutdown after the configuration done.","commit_id":"bafc3425615add09161189ea12134c71b4d9edb8"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"610a697a0f33b631ff8517262a00c6e0cc7631b1","unresolved":false,"context_lines":[{"line_number":599,"context_line":"            timeout \u003d CONF.vif_plugging_timeout"},{"line_number":600,"context_line":"            events \u003d self._get_neutron_events(network_info,"},{"line_number":601,"context_line":"                                              power_on, first_boot)"},{"line_number":602,"context_line":"            with self._virtapi.wait_for_instance_event("},{"line_number":603,"context_line":"                    instance, events, deadline\u003dtimeout,"},{"line_number":604,"context_line":"                    error_callback\u003dself._neutron_failed_callback):"},{"line_number":605,"context_line":"                LOG.debug(\"wait for instance event:%s\", events)"}],"source_content_type":"text/x-python","patch_set":29,"id":"5aef4532_212c0b98","line":602,"updated":"2016-03-10 15:15:51.000000000","message":"Didn\u0027t you just break this code by moving when you create the vifs?\n\nI think you want to create to be waiting for the events before you create the VIFs, otherwise you might miss that event.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"6f6e1dbd90f3389a3f00b3eac2b5fa0309376d98","unresolved":false,"context_lines":[{"line_number":599,"context_line":"            timeout \u003d CONF.vif_plugging_timeout"},{"line_number":600,"context_line":"            events \u003d self._get_neutron_events(network_info,"},{"line_number":601,"context_line":"                                              power_on, first_boot)"},{"line_number":602,"context_line":"            with self._virtapi.wait_for_instance_event("},{"line_number":603,"context_line":"                    instance, events, deadline\u003dtimeout,"},{"line_number":604,"context_line":"                    error_callback\u003dself._neutron_failed_callback):"},{"line_number":605,"context_line":"                LOG.debug(\"wait for instance event:%s\", events)"}],"source_content_type":"text/x-python","patch_set":29,"id":"1af94dfe_1a7ac324","line":602,"in_reply_to":"5aef4532_212c0b98","updated":"2016-03-17 09:06:49.000000000","message":"it depends on the vif creation to trigger the event.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"58b6603d4bf6becbbe0066ef6510bb4bf2c65cac","unresolved":false,"context_lines":[{"line_number":526,"context_line":"                                disk_image_type, network_info):"},{"line_number":527,"context_line":"            attach_disks(undo_mgr, vm_ref, vdis, disk_image_type)"},{"line_number":528,"context_line":"            attach_pci_devices(undo_mgr, vm_ref)"},{"line_number":529,"context_line":"            self._create_vifs(instance, vm_ref, network_info)"},{"line_number":530,"context_line":""},{"line_number":531,"context_line":"        if rescue:"},{"line_number":532,"context_line":"            # NOTE(johannes): Attach disks from original VM to rescue VM now,"}],"source_content_type":"text/x-python","patch_set":34,"id":"fa0719c6_078712b6","line":529,"updated":"2016-03-21 18:18:46.000000000","message":"I\u0027ve forgotten - can you remind me why create_vifs needs to happen here and not setup_network_step?\nWe may need a comment here or in the commit message to explain?","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"b7e1f3c9dc3ae93759aa913389e3325808eaac14","unresolved":false,"context_lines":[{"line_number":348,"context_line":"        if bad_volumes_callback and bad_devices:"},{"line_number":349,"context_line":"            bad_volumes_callback(bad_devices)"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        # Do some post start operations which needed to be done after start."},{"line_number":352,"context_line":"        self._post_start_actions(instance)"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def _post_start_actions(self, instance):"}],"source_content_type":"text/x-python","patch_set":38,"id":"1a122d0e_f4703174","line":351,"updated":"2016-04-20 13:47:11.000000000","message":"Comment isn\u0027t useful.","commit_id":"165aa3e07a70ee2265f9f965607406687052c3b7"}],"plugins/xenserver/xenapi/etc/xapi.d/plugins/nova_plugin_network":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"da85f559_f0498cd1","updated":"2015-11-09 15:28:28.000000000","message":"I think we should not add this file; can we re-use the host plugin?  We already have a very similar \"iptables_config\" call in there.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"c63285c9f90764fbef929e64b9e7d886a551ed18","unresolved":false,"context_lines":[{"line_number":18,"context_line":"#    under the License."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"#"},{"line_number":21,"context_line":"# XenAPI plugin for executing network commands (ovs, iptables, etc) on dom0"},{"line_number":22,"context_line":"#"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"import gettext"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"da85f559_b0931afa","line":21,"updated":"2015-11-09 07:00:08.000000000","message":"update the head comments.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":22,"context_line":"#"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"import gettext"},{"line_number":25,"context_line":"gettext.install(\u0027neutron\u0027, unicode\u003d1)"},{"line_number":26,"context_line":"try:"},{"line_number":27,"context_line":"    import json"},{"line_number":28,"context_line":"except ImportError:"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"da85f559_b0d0e454","line":25,"updated":"2015-11-09 15:28:28.000000000","message":"Not neutron","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":22,"context_line":"#"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"import gettext"},{"line_number":25,"context_line":"gettext.install(\u0027neutron\u0027, unicode\u003d1)"},{"line_number":26,"context_line":"try:"},{"line_number":27,"context_line":"    import json"},{"line_number":28,"context_line":"except ImportError:"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"da85f559_58cafaa2","line":25,"in_reply_to":"da85f559_b0d0e454","updated":"2015-11-09 16:00:09.000000000","message":"ah, yes. Sorry for the mistake.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                            stderr\u003dpipe, close_fds\u003dTrue)"},{"line_number":53,"context_line":"    (out, err) \u003d proc.communicate(cmd_input)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    if err:"},{"line_number":56,"context_line":"        raise PluginError(err)"},{"line_number":57,"context_line":"    return out"},{"line_number":58,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":2,"id":"da85f559_f01aec16","line":55,"updated":"2015-11-09 15:28:28.000000000","message":"This should be if proc.return_code !\u003d 0 (for Neutron as well)","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                            stderr\u003dpipe, close_fds\u003dTrue)"},{"line_number":53,"context_line":"    (out, err) \u003d proc.communicate(cmd_input)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    if err:"},{"line_number":56,"context_line":"        raise PluginError(err)"},{"line_number":57,"context_line":"    return out"},{"line_number":58,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":2,"id":"da85f559_b83c4694","line":55,"in_reply_to":"da85f559_f01aec16","updated":"2015-11-09 16:00:09.000000000","message":"agree.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"dd877c0926ee885a0384e2ea28c2ada40d93ddd1","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"def run_command(session, args):"},{"line_number":61,"context_line":"    cmd \u003d json.loads(args.get(\u0027cmd\u0027))"},{"line_number":62,"context_line":"    if cmd and cmd[0] not in ALLOWED_CMDS:"},{"line_number":63,"context_line":"        msg \u003d _(\"Dom0 execution of \u0027%s\u0027 is not permitted\") % cmd[0]"},{"line_number":64,"context_line":"        raise PluginError(msg)"},{"line_number":65,"context_line":"    result \u003d _run_command(cmd, json.loads(args.get(\u0027cmd_input\u0027, \u0027null\u0027)))"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"da85f559_3096344d","line":62,"updated":"2015-11-09 15:28:28.000000000","message":"If \u0027cmd\u0027 is \"\" or None then I suspect we\u0027d get a weird error from trying to execute below.\nWould be better to catch here + report.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7deb38bc6333f7550fce5193363433451dfe3a66","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"def run_command(session, args):"},{"line_number":61,"context_line":"    cmd \u003d json.loads(args.get(\u0027cmd\u0027))"},{"line_number":62,"context_line":"    if cmd and cmd[0] not in ALLOWED_CMDS:"},{"line_number":63,"context_line":"        msg \u003d _(\"Dom0 execution of \u0027%s\u0027 is not permitted\") % cmd[0]"},{"line_number":64,"context_line":"        raise PluginError(msg)"},{"line_number":65,"context_line":"    result \u003d _run_command(cmd, json.loads(args.get(\u0027cmd_input\u0027, \u0027null\u0027)))"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"da85f559_f8d88e1f","line":62,"in_reply_to":"da85f559_3096344d","updated":"2015-11-09 16:00:09.000000000","message":"Good point. I will add defense code to stop it and raise an error.","commit_id":"48207fe9faed8d1e404fc8a12e8af9fa82be4e86"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"af5f6db84053add4e522ce11b0bbbb2bb6c7ecd5","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/usr/bin/env python"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":"# Copyright 2015 OpenStack Foundation"},{"line_number":4,"context_line":"# Copyright 2015 United States Government as represented by the"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"da85f559_f4c34487","line":1,"updated":"2015-11-11 16:42:16.000000000","message":"Did you explore removing this file and using xenhost as suggested in the previous patch?","commit_id":"4795794e1fa23e19f28aec1e5c33ed6f716f453f"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"89b41ee323a39ef67c53ab953d258c218b393b2e","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/usr/bin/env python"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":"# Copyright 2015 OpenStack Foundation"},{"line_number":4,"context_line":"# Copyright 2015 United States Government as represented by the"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"da85f559_ffb31d98","line":1,"in_reply_to":"da85f559_f4c34487","updated":"2015-11-11 17:15:25.000000000","message":"Bob: I\u0027m so sorry that I missed that comment. Yes, we should reuse xenhost by adding a function into it.","commit_id":"4795794e1fa23e19f28aec1e5c33ed6f716f453f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"6d6934e630a45e340514c6e1daddf9df07e03c9a","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/usr/bin/env python"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":"# Copyright 2015 OpenStack Foundation"},{"line_number":4,"context_line":"# Copyright 2015 United States Government as represented by the"}],"source_content_type":"application/octet-stream","patch_set":8,"id":"da85f559_c33f2885","line":1,"updated":"2015-11-12 14:57:57.000000000","message":"Remove this file?","commit_id":"08b6a4d752a62c59cdb8f472d7ee17907c36e9a5"}],"plugins/xenserver/xenapi/etc/xapi.d/plugins/nova_plugin_version":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"6713c8a0a820dbc1169f11377a99955969538977","unresolved":false,"context_lines":[{"line_number":31,"context_line":"# 1.3 - Add vhd2 functions for doing glance operations by url"},{"line_number":32,"context_line":"# 1.4 - Add support of Glance v2 api"},{"line_number":33,"context_line":"# 1.5 - Added function for network configuration on ovs bridge"},{"line_number":34,"context_line":"PLUGIN_VERSION \u003d \"1.4\""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def get_version(session):"}],"source_content_type":"application/octet-stream","patch_set":42,"id":"5a9d85d2_7aa4c956","line":34,"updated":"2016-06-20 14:15:59.000000000","message":"This needs to be updated","commit_id":"73c12fedc06fb8e8d7b8343c5ba3157b48e4175f"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"87721c586cdf2a2f1550ae6b22ea53a8c9f43793","unresolved":false,"context_lines":[{"line_number":31,"context_line":"# 1.3 - Add vhd2 functions for doing glance operations by url"},{"line_number":32,"context_line":"# 1.4 - Add support of Glance v2 api"},{"line_number":33,"context_line":"# 1.5 - Added function for network configuration on ovs bridge"},{"line_number":34,"context_line":"PLUGIN_VERSION \u003d \"1.4\""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def get_version(session):"}],"source_content_type":"application/octet-stream","patch_set":42,"id":"5a9d85d2_9717b550","line":34,"in_reply_to":"5a9d85d2_7aa4c956","updated":"2016-06-21 00:27:43.000000000","message":"Ah, yes. thanks.","commit_id":"73c12fedc06fb8e8d7b8343c5ba3157b48e4175f"}],"plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"1d5b86d9f9a2d5a25d39dc3a3bad077a7c1b5617","unresolved":false,"context_lines":[{"line_number":473,"context_line":"            \"get_config\": get_config,"},{"line_number":474,"context_line":"            \"set_config\": set_config,"},{"line_number":475,"context_line":"            \"iptables_config\": iptables_config,"},{"line_number":476,"context_line":"            \"ovs_config\": ovs_config,"},{"line_number":477,"context_line":"            \"host_uptime\": host_uptime})"}],"source_content_type":"application/octet-stream","patch_set":15,"id":"9a8ffd7b_a96bd1f0","line":476,"updated":"2015-11-30 14:39:56.000000000","message":"\u0027network_config\u0027 rather than \u0027ovs_config\u0027 so it can be re-used more generically.\nShame that we can\u0027t rename \u0027iptables_config\u0027!","commit_id":"bdc4f60397be3b77b9566b1ba0023ff37bef5450"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"cb21b00355c81dfc43bdd33260e9611007e34280","unresolved":false,"context_lines":[{"line_number":473,"context_line":"            \"get_config\": get_config,"},{"line_number":474,"context_line":"            \"set_config\": set_config,"},{"line_number":475,"context_line":"            \"iptables_config\": iptables_config,"},{"line_number":476,"context_line":"            \"ovs_config\": ovs_config,"},{"line_number":477,"context_line":"            \"host_uptime\": host_uptime})"}],"source_content_type":"application/octet-stream","patch_set":15,"id":"9a8ffd7b_1f3c6f2e","line":476,"in_reply_to":"9a8ffd7b_a96bd1f0","updated":"2015-12-01 02:14:45.000000000","message":"Agree. Will change accordingly. thanks.","commit_id":"bdc4f60397be3b77b9566b1ba0023ff37bef5450"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":213,"context_line":"        raise pluginlib.PluginError(_(\"Invalid iptables command\"))"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"def network_config(session, args):"},{"line_number":217,"context_line":"    # function to config OVS bridge"},{"line_number":218,"context_line":"    ALLOWED_CMDS \u003d ["},{"line_number":219,"context_line":"        \u0027ovs-vsctl\u0027,"}],"source_content_type":"application/octet-stream","patch_set":29,"id":"9aed3d3a_31713c77","line":216,"updated":"2016-02-29 15:25:07.000000000","message":"network_config is nice and generic; perhaps we should enable it to run the iptables_config as well and then in future we can deprecate the specific call to iptables_config?\n\nThoughts?","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":213,"context_line":"        raise pluginlib.PluginError(_(\"Invalid iptables command\"))"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"def network_config(session, args):"},{"line_number":217,"context_line":"    # function to config OVS bridge"},{"line_number":218,"context_line":"    ALLOWED_CMDS \u003d ["},{"line_number":219,"context_line":"        \u0027ovs-vsctl\u0027,"}],"source_content_type":"application/octet-stream","patch_set":29,"id":"7af24918_1485c5cd","line":216,"in_reply_to":"9aed3d3a_31713c77","updated":"2016-03-04 11:13:28.000000000","message":"Yes, it seems more reasonable to cover iptables_config in network_config. But if we go with that, I suggest also change the calling places as well. otherwise, others may be confused.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"95aa54eed88fd91945c51f4ec0935b34203effb2","unresolved":false,"context_lines":[{"line_number":218,"context_line":"    ALLOWED_CMDS \u003d ["},{"line_number":219,"context_line":"        \u0027ovs-vsctl\u0027,"},{"line_number":220,"context_line":"        ]"},{"line_number":221,"context_line":"    cmd \u003d json.loads(args.get(\u0027cmd\u0027))"},{"line_number":222,"context_line":"    if cmd is None or cmd \u003d\u003d []:"},{"line_number":223,"context_line":"        msg \u003d _(\"empty command is supplied\")"},{"line_number":224,"context_line":"        raise pluginlib.PluginError(msg)"}],"source_content_type":"application/octet-stream","patch_set":29,"id":"9aed3d3a_b1a70cbc","line":221,"updated":"2016-02-29 15:25:07.000000000","message":"Please use pluginlib as above to safely get the args","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d76b8b7fa28a7477b70c924901b234dc2d171a0a","unresolved":false,"context_lines":[{"line_number":218,"context_line":"    ALLOWED_CMDS \u003d ["},{"line_number":219,"context_line":"        \u0027ovs-vsctl\u0027,"},{"line_number":220,"context_line":"        ]"},{"line_number":221,"context_line":"    cmd \u003d json.loads(args.get(\u0027cmd\u0027))"},{"line_number":222,"context_line":"    if cmd is None or cmd \u003d\u003d []:"},{"line_number":223,"context_line":"        msg \u003d _(\"empty command is supplied\")"},{"line_number":224,"context_line":"        raise pluginlib.PluginError(msg)"}],"source_content_type":"application/octet-stream","patch_set":29,"id":"7af24918_546bcd1d","line":221,"in_reply_to":"9aed3d3a_b1a70cbc","updated":"2016-03-04 11:13:28.000000000","message":"Done","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"610a697a0f33b631ff8517262a00c6e0cc7631b1","unresolved":false,"context_lines":[{"line_number":225,"context_line":"    if cmd[0] not in ALLOWED_CMDS:"},{"line_number":226,"context_line":"        msg \u003d _(\"Dom0 execution of \u0027%s\u0027 is not permitted\") % cmd[0]"},{"line_number":227,"context_line":"        raise pluginlib.PluginError(msg)"},{"line_number":228,"context_line":"    result \u003d _run_command(cmd, json.loads(args.get(\u0027cmd_input\u0027, \u0027null\u0027)))"},{"line_number":229,"context_line":"    return json.dumps(result)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":29,"id":"5aef4532_6170d3a4","line":228,"updated":"2016-03-10 15:15:51.000000000","message":"we probably need more care here, to make sure folks don\u0027t try run multiple commands at once here.\n\nIts a defence in depth concern, rather than any direct threat.\n\nI would rather we sent specific high level operations, in some ways, but that is probably more intrusive, I guess?","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"6f6e1dbd90f3389a3f00b3eac2b5fa0309376d98","unresolved":false,"context_lines":[{"line_number":225,"context_line":"    if cmd[0] not in ALLOWED_CMDS:"},{"line_number":226,"context_line":"        msg \u003d _(\"Dom0 execution of \u0027%s\u0027 is not permitted\") % cmd[0]"},{"line_number":227,"context_line":"        raise pluginlib.PluginError(msg)"},{"line_number":228,"context_line":"    result \u003d _run_command(cmd, json.loads(args.get(\u0027cmd_input\u0027, \u0027null\u0027)))"},{"line_number":229,"context_line":"    return json.dumps(result)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":29,"id":"1af94dfe_7a5667d3","line":228,"in_reply_to":"5aef4532_6170d3a4","updated":"2016-03-17 09:06:49.000000000","message":"Agree. In the next version source code, I will try to sent the hihg level operations via the plugin invoking and execut the real command in the plugin.","commit_id":"52d0d428576ef42cce22380c8780b57c64707506"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"58b6603d4bf6becbbe0066ef6510bb4bf2c65cac","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        \u0027ovs_set_if_external_id\u0027,"},{"line_number":227,"context_line":"        ]"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    class network_cmd(object):"},{"line_number":230,"context_line":"        def __init__(self, cmd_name, cmd_args):"},{"line_number":231,"context_line":"            self.cmd_name \u003d cmd_name"},{"line_number":232,"context_line":"            self.cmd_args \u003d cmd_args"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"fa0719c6_1db941ea","line":229,"updated":"2016-03-21 18:18:46.000000000","message":"This doesn\u0027t need to be function-scoped; could it be top level?\nPerhaps call it XenNetworkCmd?","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"5ab30e8dc38c24303c283c29e64f9e46d0c0cae7","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        \u0027ovs_set_if_external_id\u0027,"},{"line_number":227,"context_line":"        ]"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    class network_cmd(object):"},{"line_number":230,"context_line":"        def __init__(self, cmd_name, cmd_args):"},{"line_number":231,"context_line":"            self.cmd_name \u003d cmd_name"},{"line_number":232,"context_line":"            self.cmd_args \u003d cmd_args"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"fa0719c6_2a760fa4","line":229,"in_reply_to":"fa0719c6_1db941ea","updated":"2016-03-23 07:43:51.000000000","message":"Done","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"58b6603d4bf6becbbe0066ef6510bb4bf2c65cac","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            self.cmd_args \u003d cmd_args"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        def run(self):"},{"line_number":235,"context_line":"            cmd_fun \u003d getattr(self, \u0027_%s\u0027 % self.cmd_name, None)"},{"line_number":236,"context_line":"            if not cmd_fun:"},{"line_number":237,"context_line":"                raise pluginlib.PluginError("},{"line_number":238,"context_line":"                    _(\"Undefined function %s\") % cmd_fun)"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"fa0719c6_8a796996","line":235,"updated":"2016-03-21 18:18:46.000000000","message":"Do we expect more functions to be added?\nI\u0027m wondering if these 4 functions should be top-level plugin methods?\n\nIf we keep as function methods here, perhaps have:\nALLOWED_CMDS: {\u0027ovs_add_patch_port\u0027: ovs_add_patch_port}\nthen you could use:\nALLOWED_CMDS[self.cmd_name](**args)\nwith each function taking named arguments?","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"5ab30e8dc38c24303c283c29e64f9e46d0c0cae7","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            self.cmd_args \u003d cmd_args"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        def run(self):"},{"line_number":235,"context_line":"            cmd_fun \u003d getattr(self, \u0027_%s\u0027 % self.cmd_name, None)"},{"line_number":236,"context_line":"            if not cmd_fun:"},{"line_number":237,"context_line":"                raise pluginlib.PluginError("},{"line_number":238,"context_line":"                    _(\"Undefined function %s\") % cmd_fun)"}],"source_content_type":"application/octet-stream","patch_set":34,"id":"fa0719c6_ca46734e","line":235,"in_reply_to":"fa0719c6_8a796996","updated":"2016-03-23 07:43:51.000000000","message":"Done","commit_id":"cfbe0d2ca93392a42a3f744d4cb598d4414125af"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"7e4f54190e7ef3eb8e1397de07c7a782bc9e0333","unresolved":false,"context_lines":[{"line_number":261,"context_line":"    }"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"def network_config(session, args):"},{"line_number":265,"context_line":"    \"\"\"network config functions\"\"\""},{"line_number":266,"context_line":"    cmd \u003d pluginlib.exists(args, \u0027cmd\u0027)"},{"line_number":267,"context_line":"    if not isinstance(cmd, basestring):"}],"source_content_type":"application/octet-stream","patch_set":37,"id":"fa0719c6_2dc73365","line":264,"updated":"2016-03-23 13:07:55.000000000","message":"This looks much cleaner than it was - thanks!","commit_id":"1362a71c024564a21c2b6eac62c86a2665458df6"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"7e4f54190e7ef3eb8e1397de07c7a782bc9e0333","unresolved":false,"context_lines":[{"line_number":265,"context_line":"    \"\"\"network config functions\"\"\""},{"line_number":266,"context_line":"    cmd \u003d pluginlib.exists(args, \u0027cmd\u0027)"},{"line_number":267,"context_line":"    if not isinstance(cmd, basestring):"},{"line_number":268,"context_line":"        msg \u003d _(\"invalid command \u0027%s\u0027\") % cmd"},{"line_number":269,"context_line":"        raise pluginlib.PluginError(msg)"},{"line_number":270,"context_line":"        return"},{"line_number":271,"context_line":"    if cmd not in ALLOWED_NETWORK_CMDS:"}],"source_content_type":"application/octet-stream","patch_set":37,"id":"fa0719c6_4d8f072a","line":268,"updated":"2016-03-23 13:07:55.000000000","message":"If it\u0027s not a string, then we probably can\u0027t format it as a string :)  Would definitely need to be wrapped as str(cmd)","commit_id":"1362a71c024564a21c2b6eac62c86a2665458df6"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"7f05e717d7486973e057242a91120ab42cff4519","unresolved":false,"context_lines":[{"line_number":265,"context_line":"    \"\"\"network config functions\"\"\""},{"line_number":266,"context_line":"    cmd \u003d pluginlib.exists(args, \u0027cmd\u0027)"},{"line_number":267,"context_line":"    if not isinstance(cmd, basestring):"},{"line_number":268,"context_line":"        msg \u003d _(\"invalid command \u0027%s\u0027\") % cmd"},{"line_number":269,"context_line":"        raise pluginlib.PluginError(msg)"},{"line_number":270,"context_line":"        return"},{"line_number":271,"context_line":"    if cmd not in ALLOWED_NETWORK_CMDS:"}],"source_content_type":"application/octet-stream","patch_set":37,"id":"fa0719c6_00b91c2e","line":268,"in_reply_to":"fa0719c6_4d8f072a","updated":"2016-03-24 10:20:50.000000000","message":"Done","commit_id":"1362a71c024564a21c2b6eac62c86a2665458df6"}]}