)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6d0253dbfd02718c4f80dec068d6b177fabe1e3d","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix LibvirtGenericVIFDriver.get_config() for quota"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Include VIF quotas into the config also for network and direct types, not only"},{"line_number":10,"context_line":"for bridge type."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I0f0111fc79fe90900e38df022034b208f1129088"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAATn%2F%2BXnc%3D","line":9,"updated":"2013-11-26 20:38:00.000000000","message":"I know there is some information in the bug report, but I\u0027d really like to see some more details in the commit message itself on what the problem is and how this solves it.\n\nhttps://wiki.openstack.org/wiki/GitCommitMessages#Information_in_commit_messages\n\nI think it\u0027s important to point out why some vif types need to change while others don\u0027t.  This may be obvious to those familiar with the different vif types, but for me it\u0027s not.","commit_id":"80bb1424fdd10ab5b160098e94d028ddd1a63c44"}],"nova/tests/virt/libvirt/test_libvirt_vif.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0ea66b35049e912cb86b8cb6c9fdec9147c78123","unresolved":false,"context_lines":[{"line_number":357,"context_line":""},{"line_number":358,"context_line":"    def test_model_qemu(self):"},{"line_number":359,"context_line":"        for vif_obj in ("},{"line_number":360,"context_line":"            self.vif_bridge, self.vif_ovs, self.vif_ivs, self.vif_8021qbh,"},{"line_number":361,"context_line":"            self.vif_8021qbg, self.vif_mlnx, self.vif_midonet"},{"line_number":362,"context_line":"        ):"},{"line_number":363,"context_line":"            self.flags(libvirt_use_virtio_for_bridges\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2Bfh4%3D","line":360,"updated":"2013-11-25 14:40:53.000000000","message":"I like what you\u0027re doing here.  I don\u0027t see iovisor in this case though, but that\u0027s probably because there is no self.vif_iovisor in this test class, so is it not tested?  Would be good to cover that also.  Looks like there are some plug/unplug tests for iovisor and the test_generic_iovisor_driver test uses self.vif_ivs.  I\u0027m not sure if I\u0027m missing something or not.","commit_id":"c3f85809f5daf28ea72f8abbd7b7b5f470ac6ef3"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"bc64f19fecc7e489be72afb75d5a16da26e93855","unresolved":false,"context_lines":[{"line_number":380,"context_line":"            self.assertEqual(outbound.get(\"average\"), \"10\")"},{"line_number":381,"context_line":"            self.assertEqual(outbound.get(\"peak\"), \"20\")"},{"line_number":382,"context_line":"            self.assertEqual(outbound.get(\"burst\"), \"30\")"},{"line_number":383,"context_line":""},{"line_number":384,"context_line":"            self._assertModel(xml, \"virtio\", \"qemu\")"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    def test_model_xen(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BfeM%3D","line":383,"updated":"2013-11-25 14:56:41.000000000","message":"Instead of repeating the values here, couldn\u0027t you use bandwith[\u0027quota:vif_x_y\u0027] ? You\u0027d have to rename the local \u0027bandwidth\u0027 variable, obviously.","commit_id":"c3f85809f5daf28ea72f8abbd7b7b5f470ac6ef3"}],"nova/virt/libvirt/vif.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0ea66b35049e912cb86b8cb6c9fdec9147c78123","unresolved":false,"context_lines":[{"line_number":373,"context_line":"            raise exception.NovaException("},{"line_number":374,"context_line":"                _(\"Unexpected vif_type\u003d%s\") % vif_type)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        designer.set_vif_bandwidth_config(conf, inst_type)"},{"line_number":377,"context_line":"        return conf"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"    def plug_bridge(self, instance, vif):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BfkI%3D","line":376,"updated":"2013-11-25 14:40:53.000000000","message":"The only problem I see with this is if someone calls the specific typed get_config_* methods rather than this convenience method that checks the types and calls the specific get_config method for us, they won\u0027t have the vif bandwidth config set.  However, I\u0027m not crazy about calling set_vif_bandwidth_config in each type-specific method either.  Thoughts?  Maybe the type-specific methods should not be public and a caller should only get to them via this general method.","commit_id":"c3f85809f5daf28ea72f8abbd7b7b5f470ac6ef3"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"8dda60255f97859e8f341c166aeaf684ad1fd138","unresolved":false,"context_lines":[{"line_number":373,"context_line":"            raise exception.NovaException("},{"line_number":374,"context_line":"                _(\"Unexpected vif_type\u003d%s\") % vif_type)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        designer.set_vif_bandwidth_config(conf, inst_type)"},{"line_number":377,"context_line":"        return conf"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"    def plug_bridge(self, instance, vif):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2Bfas%3D","line":376,"in_reply_to":"AAAATn%2F%2BfkI%3D","updated":"2013-11-25 15:04:01.000000000","message":"The libvirt vif bandwidth XML only applies to \u003cinterface\u003e elements using type\u003d\"network|bridge|direct\". In particular it won\u0027t work with type\u003dethernet, which some of the Nova vif types use.\n\nSo calling set_vif_bandwidth_config here is inappropriate - you should only call in for the configs where it is actually able to work.","commit_id":"c3f85809f5daf28ea72f8abbd7b7b5f470ac6ef3"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8a97df4a8748981cb389ad4b3b22c56193a447db","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                                                virt\u003dCONF.libvirt.virt_type)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":131,"context_line":"            conf, vif[\u0027address\u0027], model, driver)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        return conf"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BPGc%3D","line":131,"updated":"2013-11-28 14:00:56.000000000","message":"Any reason not to set the designer.set_vif_bandwidth_config(conf, inst_type) here - then you will not have to do it in each and every vif type.\nIs that an option?","commit_id":"497bdfec93d696c7200a71b85a0680d99ce07bb9"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"bcfbe06d7e38b386a1ad6e15edc9e3e35e420007","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                                                virt\u003dCONF.libvirt.virt_type)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":131,"context_line":"            conf, vif[\u0027address\u0027], model, driver)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        return conf"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F9oCo%3D","line":131,"in_reply_to":"AAAATn%2F%2BIPU%3D","updated":"2013-12-09 15:17:53.000000000","message":"@garyk: what do you think?","commit_id":"497bdfec93d696c7200a71b85a0680d99ce07bb9"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"f33bf3ec1cc440e0d06a06aa9df7e895549f0030","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                                                virt\u003dCONF.libvirt.virt_type)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":131,"context_line":"            conf, vif[\u0027address\u0027], model, driver)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        return conf"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BIPU%3D","line":131,"in_reply_to":"AAAATn%2F%2BIY8%3D","updated":"2013-12-02 09:13:15.000000000","message":"I think the code is best as you have it now.","commit_id":"497bdfec93d696c7200a71b85a0680d99ce07bb9"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"ac173b93fbc950f81a1e9383c9a5ba31373d7ece","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                                                virt\u003dCONF.libvirt.virt_type)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":131,"context_line":"            conf, vif[\u0027address\u0027], model, driver)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        return conf"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BIY8%3D","line":131,"in_reply_to":"AAAATn%2F%2BO%2F8%3D","updated":"2013-12-02 08:22:03.000000000","message":"The caller should not have to care of which network driver is used. libvirt/vif.py is the best place to decide if XML tag \u003cbandwidth\u003e should be generated.","commit_id":"497bdfec93d696c7200a71b85a0680d99ce07bb9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5e739212bf2b3ebdaa25b388cb493591bff0fe88","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                                                virt\u003dCONF.libvirt.virt_type)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":131,"context_line":"            conf, vif[\u0027address\u0027], model, driver)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        return conf"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BO%2F8%3D","line":131,"in_reply_to":"AAAATn%2F%2BPA0%3D","updated":"2013-11-28 14:26:20.000000000","message":"Why not just pass an extra parameter to def get_config(self, instance, vif, image_meta, inst_type),\nfor example:\ndef get_config(self, instance, vif, image_meta, inst_type, set_bw):\nThis can be True/False.\nDoes that provide a solution?","commit_id":"497bdfec93d696c7200a71b85a0680d99ce07bb9"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"8cadf0a1b0df053fbb5a057a07a249f39852d612","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                                                virt\u003dCONF.libvirt.virt_type)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":131,"context_line":"            conf, vif[\u0027address\u0027], model, driver)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        return conf"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BPA0%3D","line":131,"in_reply_to":"AAAATn%2F%2BPDo%3D","updated":"2013-11-28 14:22:15.000000000","message":"\u003e Why not have a flag then - at least treat this on one place instead of having to copy lines of code.\n\nPlease read this previous comment of \tMatt Riedemann:\nhttps://review.openstack.org/#/c/58290/2/nova/virt/libvirt/vif.py\n\nI would also prefer to only call designer.set_vif_bandwidth_config() in one function, but first the code should be refactored to make get_config_xxx() functions private which is out of the scope of my patch.","commit_id":"497bdfec93d696c7200a71b85a0680d99ce07bb9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7ae71303371b2d30e48d8c96ca7c639acdd02532","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                                                virt\u003dCONF.libvirt.virt_type)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":131,"context_line":"            conf, vif[\u0027address\u0027], model, driver)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        return conf"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BPDo%3D","line":131,"in_reply_to":"AAAATn%2F%2BPEU%3D","updated":"2013-11-28 14:06:31.000000000","message":"Why not have a flag then - at least treat this on one place instead of having to copy lines of code.","commit_id":"497bdfec93d696c7200a71b85a0680d99ce07bb9"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"7875cc628ed9697c2161be51077685b754b4c81a","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                                                virt\u003dCONF.libvirt.virt_type)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":131,"context_line":"            conf, vif[\u0027address\u0027], model, driver)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        return conf"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BPEU%3D","line":131,"in_reply_to":"AAAATn%2F%2BPGc%3D","updated":"2013-11-28 14:03:13.000000000","message":"See previous comments on this review - not all libvirt vif types support bandwidth controls, so you must not unconditionally set it for all.","commit_id":"497bdfec93d696c7200a71b85a0680d99ce07bb9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8a97df4a8748981cb389ad4b3b22c56193a447db","unresolved":false,"context_lines":[{"line_number":188,"context_line":"                                      image_meta, inst_type)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        dev \u003d self.get_vif_devname(vif)"},{"line_number":191,"context_line":"        designer.set_vif_host_backend_ethernet_config(conf, dev)"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"        return conf"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BPHc%3D","line":191,"updated":"2013-11-28 14:00:56.000000000","message":"why is this not set here?\n\nBut this is not relevant if we do what is suggest above","commit_id":"497bdfec93d696c7200a71b85a0680d99ce07bb9"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"ac173b93fbc950f81a1e9383c9a5ba31373d7ece","unresolved":false,"context_lines":[{"line_number":188,"context_line":"                                      image_meta, inst_type)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        dev \u003d self.get_vif_devname(vif)"},{"line_number":191,"context_line":"        designer.set_vif_host_backend_ethernet_config(conf, dev)"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"        return conf"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BIYk%3D","line":191,"in_reply_to":"AAAATn%2F%2BPHc%3D","updated":"2013-12-02 08:22:03.000000000","message":"Please read this previous comment of Daniel Berrange: https://review.openstack.org/#/c/58290/2/nova/virt/libvirt/vif.py\n\n@Daniel, Matt: So what do you think?\n\n@garyk: Do you keep your -1 vote? If yes, what should I do to fix my patch?","commit_id":"497bdfec93d696c7200a71b85a0680d99ce07bb9"}]}