)]}'
{"nova/tests/fakelibvirt.py":[{"author":{"_account_id":105,"name":"Kyle Mestery","email":"mestery@mestery.com","username":"mestery"},"change_message_id":"d9d1f9fad484b2207187dfa08d13c3a4ed3e831b","unresolved":false,"context_lines":[{"line_number":503,"context_line":"        self._id_counter \u003d 1  # libvirt reserves 0 for the hypervisor."},{"line_number":504,"context_line":"        self._nwfilters \u003d {}"},{"line_number":505,"context_line":"        self.fakeLibVersion \u003d 9007"},{"line_number":506,"context_line":"        self.fakeVersion \u003d 9007"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"    def _add_filter(self, nwfilter):"},{"line_number":509,"context_line":"        self._nwfilters[nwfilter._name] \u003d nwfilter"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fes4%3D","line":506,"updated":"2013-01-19 20:51:05.000000000","message":"Just curious, but the version used to be reported as 14000 here, why the change to 9007?","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"364ebdea8d7c343795afce8910ec7d75c7e1325b","unresolved":false,"context_lines":[{"line_number":503,"context_line":"        self._id_counter \u003d 1  # libvirt reserves 0 for the hypervisor."},{"line_number":504,"context_line":"        self._nwfilters \u003d {}"},{"line_number":505,"context_line":"        self.fakeLibVersion \u003d 9007"},{"line_number":506,"context_line":"        self.fakeVersion \u003d 9007"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"    def _add_filter(self, nwfilter):"},{"line_number":509,"context_line":"        self._nwfilters[nwfilter._name] \u003d nwfilter"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2FeDs%3D","line":506,"in_reply_to":"AAAAM3%2F%2FeGM%3D","updated":"2013-01-21 11:15:28.000000000","message":"It would be better to add a comment on it.","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"78bc45ea2f39cb9ad6b7699567d3828e1c7da925","unresolved":false,"context_lines":[{"line_number":503,"context_line":"        self._id_counter \u003d 1  # libvirt reserves 0 for the hypervisor."},{"line_number":504,"context_line":"        self._nwfilters \u003d {}"},{"line_number":505,"context_line":"        self.fakeLibVersion \u003d 9007"},{"line_number":506,"context_line":"        self.fakeVersion \u003d 9007"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"    def _add_filter(self, nwfilter):"},{"line_number":509,"context_line":"        self._nwfilters[nwfilter._name] \u003d nwfilter"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2FeGM%3D","line":506,"in_reply_to":"AAAAM3%2F%2Fes4%3D","updated":"2013-01-21 09:51:57.000000000","message":"14000 is a not version that libvirt has used - I picked 9007 as a real libvirt historical version number, which works with the tests.","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"1dfe7b2939641a3f5b63727628cb924ddbbac441","unresolved":false,"context_lines":[{"line_number":460,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":461,"context_line":"        for (network, mapping) in network_info:"},{"line_number":462,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":463,"context_line":"                                 self._conn)"},{"line_number":464,"context_line":""},{"line_number":465,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":466,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2FvrU%3D","line":463,"updated":"2013-01-07 22:06:48.000000000","message":"please see previous comments about this - why is it not set in the __int__?\nyou can save all of the ifs and do not need the api change for plug\n\ni seems to have reviewed an abandoned path :(","commit_id":"29a10b70b82a4bdc50ead48fc7732c9e8b48c100"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a4af0a54d90620bee775ed12b3daf276dac6fe96","unresolved":false,"context_lines":[{"line_number":460,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":461,"context_line":"        for (network, mapping) in network_info:"},{"line_number":462,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":463,"context_line":"                                 self._conn)"},{"line_number":464,"context_line":""},{"line_number":465,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":466,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2FvDA%3D","line":463,"updated":"2013-01-08 07:29:58.000000000","message":"Not sure if you have seen my previous comment - I do not think that we should pass this as a parameter. On init we can learn the libvurt version and then use it internally. \nIn addition to this in the plug and uplug a function needs to be called each time to get the libvirt version","commit_id":"af5b82ed891f71631963bbda3f2ac9ddc3eb02d6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"bb0c9e411454eabedda6cbf33a7c4e340d7a28c5","unresolved":false,"context_lines":[{"line_number":460,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":461,"context_line":"        for (network, mapping) in network_info:"},{"line_number":462,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":463,"context_line":"                                 self._conn)"},{"line_number":464,"context_line":""},{"line_number":465,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":466,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2Fsp4%3D","line":463,"in_reply_to":"AAAAM3%2F%2FunE%3D","updated":"2013-01-11 14:10:45.000000000","message":"why does it make the code larger? you are adding in a extra parameter which is to support a old version (which in most cases is not even used and may even be deprecated).\nSounds very odd that you are not willing to address this.","commit_id":"af5b82ed891f71631963bbda3f2ac9ddc3eb02d6"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e25f28fa0fa8190c520910a3a6ed9edbc25a8329","unresolved":false,"context_lines":[{"line_number":460,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":461,"context_line":"        for (network, mapping) in network_info:"},{"line_number":462,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":463,"context_line":"                                 self._conn)"},{"line_number":464,"context_line":""},{"line_number":465,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":466,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2FunE%3D","line":463,"in_reply_to":"AAAAM3%2F%2FvDA%3D","updated":"2013-01-08 15:06:03.000000000","message":"Moving this to a constructor doesn\u0027t do anything to simplify the code - it actually makes it larger. In the context of how this code is used it also doesn\u0027t have any measurable performance benefit to cache the libvirt version number check, So I\u0027m not going to change this.","commit_id":"af5b82ed891f71631963bbda3f2ac9ddc3eb02d6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f23d610a013140f95da6897b20cd5b7714cc319e","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":470,"context_line":"        for (network, mapping) in network_info:"},{"line_number":471,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":472,"context_line":"                                 self._conn)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":475,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fd7A%3D","line":472,"updated":"2013-01-21 13:05:13.000000000","message":"I am still struggling to understand why we need the conn parameter.\n1. This is only for an outdated libvirt version. Why not just enable the user to bind a different driver when the specific libvirt version is not used. This can be done automatically at boot. \n2. In stable folsom the default driver is for the OVS is LibvirtHybridOVSBridgeDriver\nYou state that it will all be deprecated in the H version but you add a new parameter to a function that uses a version that is most probably not used. It does not make sense.","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"6a3d0f8ee9e8e44ab21e823baee9c4ba014dbfc6","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":470,"context_line":"        for (network, mapping) in network_info:"},{"line_number":471,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":472,"context_line":"                                 self._conn)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":475,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2FcjQ%3D","line":472,"in_reply_to":"AAAAM3%2F%2Fd1s%3D","updated":"2013-01-22 08:28:55.000000000","message":"I have looked at the later patches and the functionality of the LibvirtOpenVswitchDriver driver changes. Prior to your patch it will create an ethernet device irrespective of the libvirt version. With this patch if the libvirt version is updated then it will no longer create the tap device - this will be done by libvirt. I am not saying that this is good or bad, what I am saying is that it changes the functionality.\nIs this something that we want to force upon the user?","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"f8dd4876ba3883c4e60a7324c21f4743e14984de","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":470,"context_line":"        for (network, mapping) in network_info:"},{"line_number":471,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":472,"context_line":"                                 self._conn)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":475,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fd1s%3D","line":472,"in_reply_to":"AAAAM3%2F%2Fd38%3D","updated":"2013-01-21 14:30:34.000000000","message":"\u003e 1. changing the application behaviour - you did not address this. 2\n\nYes, I did - as I said above, this is fully addressed by the later patch.\n\n\u003e 2. additional parameter\n\u003e What I am saying is that you know by the configured driver which plug to use and you do not need to learn it at run time. \n\nYes we do - the whole point is to *not* rely on the users choice of configured driver. The get_config() method accepts a set of parameters which it uses to determine configuration, of which conn is one possibility. Not all VIF drivers need to use all pieces of information passed in, which is fine - they simply use information that is relevant to their needs.","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"e76dcdf970a55662bd6283bccf3b9dda52ca6f81","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":470,"context_line":"        for (network, mapping) in network_info:"},{"line_number":471,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":472,"context_line":"                                 self._conn)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":475,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fd38%3D","line":472,"in_reply_to":"AAAAM3%2F%2Fd4Y%3D","updated":"2013-01-21 14:16:35.000000000","message":"There were 2 items mentioned above:\n1. changing the application behaviour - you did not address this.\n2. additional parameter - the parameter conn is used to deretmine which plug to use - plug_ovs_bridge or plug_ovs_bridge or plug_ovs_ethernet (which you also pass the conn variable and it is not used). What I am saying is that you know by the configured driver which plug to use and you do not need to learn it at run time. \n\nAdding extra parameters is a few extra cycles. Yes, in the greater picture of things this may not make a difference, but this can be done without this parameter.","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"33c8d9ad79dea8c7d9e63d489de9252c143e9f32","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":470,"context_line":"        for (network, mapping) in network_info:"},{"line_number":471,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":472,"context_line":"                                 self._conn)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":475,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fd4Y%3D","line":472,"in_reply_to":"AAAAM3%2F%2Fd4g%3D","updated":"2013-01-21 14:00:42.000000000","message":"If you look at the later patch in the series, you\u0027ll see the back compat support is more fully addressed there - all the existing VIF drivers are switch to inherit from GenericVIFDriver and then directly call the impl that corresponded to their historical behaviour. https://review.openstack.org/#/c/19127/8/nova/virt/libvirt/vif.py While it would be possible to re-arrange this intermediate patch to allow that, it would make the diff here much larger and harder to review, so I don\u0027t think it is worth it.\n\nThe con parameter is most definitely used \u0026 it is perfectly acceptable to pass.","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"aba6afcfeb6b01cdc1af14960d1bfdc8f40fca50","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":470,"context_line":"        for (network, mapping) in network_info:"},{"line_number":471,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":472,"context_line":"                                 self._conn)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":475,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fd4g%3D","line":472,"in_reply_to":"AAAAM3%2F%2Fd5I%3D","updated":"2013-01-21 13:52:09.000000000","message":"There are two issues here:\n1. Until this patch the admin would decide the logic by configuring the driver. This is \"broken\" by the patch in the sense that if the libvirt is updated but the libvirt driver is not updated then the applications behaviour will be different.\n2. Passing a new parameter which is most probably not used. \nIn my opinion both are relevant and should be addressed.","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"a70cd68df7fff7be473322c336ea564b1394f10a","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":470,"context_line":"        for (network, mapping) in network_info:"},{"line_number":471,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":472,"context_line":"                                 self._conn)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":475,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fd5I%3D","line":472,"in_reply_to":"AAAAM3%2F%2Fd5s%3D","updated":"2013-01-21 13:37:44.000000000","message":"I think it is better to have all the logic for deciding what config to use in the same place, rather than arbitrarily splitting it across the constructor \u0026 the get_config methods. There is no performance problem or functional improvement here that justifies the approach you are suggesting.","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"72f55d76b0b966cb78bc0ec87d9ea05f3e742df5","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":470,"context_line":"        for (network, mapping) in network_info:"},{"line_number":471,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":472,"context_line":"                                 self._conn)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":475,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fd5s%3D","line":472,"in_reply_to":"AAAAM3%2F%2Fd6o%3D","updated":"2013-01-21 13:32:24.000000000","message":"I understand the purpose of the patch. I just think that it can be done better and more optimal.\nThe flag that you are passing in conjunction with the libvirt version is used to decide on the way in which the vif is plugged. [Note: this may break someones deployment if they still want the driver to function as it did irrespective of the libvirt version].\nI think that you can do this when the functions are loaded at boot time and not have to do it with each plug.\nAt the moment the only driver that does this is LibvirtOpenVswitchDriver. So why not just determine this when the aforementioned driver is loaded. What I do not understand is what for each plug call this data needs to be validated when it is not necessary.\nThe above does not have anything to do with the refactoring or its purpose. It is about having all the information and just checking it once instead of with each call.","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"13cdc1f228495eba10936836ca0ccf6816bec465","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        \"\"\"Plug VIFs into networks.\"\"\""},{"line_number":470,"context_line":"        for (network, mapping) in network_info:"},{"line_number":471,"context_line":"            self.vif_driver.plug(instance, (network, mapping),"},{"line_number":472,"context_line":"                                 self._conn)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def unplug_vifs(self, instance, network_info):"},{"line_number":475,"context_line":"        \"\"\"Unplug VIFs from networks.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAM3%2F%2Fd6o%3D","line":472,"in_reply_to":"AAAAM3%2F%2Fd7A%3D","updated":"2013-01-21 13:14:52.000000000","message":"The whole goal of this series is to only have one single VIF driver and not require the admin to make these kind of decisions. Passing in the connection lets the VIF driver automatically determine the optimal approach to use based on the libvirt connection\u0027s version / capabilities. The deprecation of existing VIF drivers is completely independent of the need to support the old style libvirt OVS setup. The intent of this refactoring is to maintain full compatibility of existing deployment scenarios, and this implies continued support for older libvirt versions, in the context of the new VIF driver impl.","commit_id":"3e6085ce72ec49754ef4aa2625ec1e7050e51c1f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5d2efc9ccafce004e934f650c614e3ffb55147fc","unresolved":false,"context_lines":[{"line_number":284,"context_line":"            self.virtapi,"},{"line_number":285,"context_line":"            get_connection\u003dself._get_connection)"},{"line_number":286,"context_line":"        vif_class \u003d importutils.import_class(CONF.libvirt_vif_driver)"},{"line_number":287,"context_line":"        LOG.error(\"Got %s \" % vif_class)"},{"line_number":288,"context_line":"        self.vif_driver \u003d vif_class(self._get_connection)"},{"line_number":289,"context_line":"        self.volume_drivers \u003d {}"},{"line_number":290,"context_line":"        for driver_str in CONF.libvirt_volume_drivers:"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAM3%2F%2FcLo%3D","line":287,"updated":"2013-01-22 15:27:50.000000000","message":"translation support","commit_id":"65aa630932af28bd8630498e6900414e90913c9b"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e9e5da6056548e3ac502a59b426fb5b3df2cfba1","unresolved":false,"context_lines":[{"line_number":284,"context_line":"            self.virtapi,"},{"line_number":285,"context_line":"            get_connection\u003dself._get_connection)"},{"line_number":286,"context_line":"        vif_class \u003d importutils.import_class(CONF.libvirt_vif_driver)"},{"line_number":287,"context_line":"        LOG.error(\"Got %s \" % vif_class)"},{"line_number":288,"context_line":"        self.vif_driver \u003d vif_class(self._get_connection)"},{"line_number":289,"context_line":"        self.volume_drivers \u003d {}"},{"line_number":290,"context_line":"        for driver_str in CONF.libvirt_volume_drivers:"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAM3%2F%2FcIw%3D","line":287,"in_reply_to":"AAAAM3%2F%2FcLo%3D","updated":"2013-01-22 15:57:45.000000000","message":"Doh, debug line that should not have been left in","commit_id":"65aa630932af28bd8630498e6900414e90913c9b"}],"nova/virt/libvirt/vif.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a4af0a54d90620bee775ed12b3daf276dac6fe96","unresolved":false,"context_lines":[{"line_number":159,"context_line":"                                      network,"},{"line_number":160,"context_line":"                                      mapping,"},{"line_number":161,"context_line":"                                      conn)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        if conn.getLibVersion() \u003e\u003d LIBVIRT_OVS_VPORT_VERSION:"},{"line_number":164,"context_line":"            self.set_config_ovs_bridge(conf, network, mapping)"},{"line_number":165,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2FvC8%3D","line":162,"updated":"2013-01-08 07:29:58.000000000","message":"Consider reading this on init","commit_id":"af5b82ed891f71631963bbda3f2ac9ddc3eb02d6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"bb0c9e411454eabedda6cbf33a7c4e340d7a28c5","unresolved":false,"context_lines":[{"line_number":159,"context_line":"                                      network,"},{"line_number":160,"context_line":"                                      mapping,"},{"line_number":161,"context_line":"                                      conn)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        if conn.getLibVersion() \u003e\u003d LIBVIRT_OVS_VPORT_VERSION:"},{"line_number":164,"context_line":"            self.set_config_ovs_bridge(conf, network, mapping)"},{"line_number":165,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2Fspg%3D","line":162,"in_reply_to":"AAAAM3%2F%2Fum4%3D","updated":"2013-01-11 14:10:45.000000000","message":"The change is only relevant for libvirt versions less than 0.9.11. You can support this by just having the user bind to the same libvirt function as before an implement the plug and unplug and indicate that it will be deprecated in H version. You are adding a new function parameter for a edge case.","commit_id":"af5b82ed891f71631963bbda3f2ac9ddc3eb02d6"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e25f28fa0fa8190c520910a3a6ed9edbc25a8329","unresolved":false,"context_lines":[{"line_number":159,"context_line":"                                      network,"},{"line_number":160,"context_line":"                                      mapping,"},{"line_number":161,"context_line":"                                      conn)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        if conn.getLibVersion() \u003e\u003d LIBVIRT_OVS_VPORT_VERSION:"},{"line_number":164,"context_line":"            self.set_config_ovs_bridge(conf, network, mapping)"},{"line_number":165,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2Fum4%3D","line":162,"in_reply_to":"AAAAM3%2F%2FvC8%3D","updated":"2013-01-08 15:06:03.000000000","message":"Per my other comment I don\u0027t think such a change is worthwhile.","commit_id":"af5b82ed891f71631963bbda3f2ac9ddc3eb02d6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a4af0a54d90620bee775ed12b3daf276dac6fe96","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        except exception.ProcessExecutionError:"},{"line_number":195,"context_line":"            LOG.exception(_(\"Failed while unplugging vif\"), instance\u003dinstance)"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    def unplug_ovs_bridge(self, instance, vif):"},{"line_number":198,"context_line":"        pass"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def unplug(self, instance, vif, conn):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2FvC4%3D","line":197,"updated":"2013-01-08 07:29:58.000000000","message":"why not define this in the base class?","commit_id":"af5b82ed891f71631963bbda3f2ac9ddc3eb02d6"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e25f28fa0fa8190c520910a3a6ed9edbc25a8329","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        except exception.ProcessExecutionError:"},{"line_number":195,"context_line":"            LOG.exception(_(\"Failed while unplugging vif\"), instance\u003dinstance)"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    def unplug_ovs_bridge(self, instance, vif):"},{"line_number":198,"context_line":"        pass"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def unplug(self, instance, vif, conn):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAM3%2F%2Fum8%3D","line":197,"in_reply_to":"AAAAM3%2F%2FvC4%3D","updated":"2013-01-08 15:06:03.000000000","message":"The LibvirtBaseVIFDriver class merely defines the API contract for the driver to use so I don\u0027t think it is appropriate to move this method.","commit_id":"af5b82ed891f71631963bbda3f2ac9ddc3eb02d6"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"change_message_id":"9e664e5137a2414bee1f0513828567248f024736","unresolved":false,"context_lines":[{"line_number":346,"context_line":"        LOG.warn(_(\"LibvirtOpenVswitchDriver is deprecated and \""},{"line_number":347,"context_line":"                   \"will be removed in the Hxxxx release. Please \""},{"line_number":348,"context_line":"                   \"update the \u0027libvirt_vif_driver\u0027 config parameter \""},{"line_number":349,"context_line":"                   \"to use the LibvirtGenericVIFDriver class instead\"))"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def get_bridge_name(self, network):"},{"line_number":352,"context_line":"        return network.get(\u0027bridge\u0027) or CONF.libvirt_ovs_bridge"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAM3%2F%2Fa0Q%3D","line":349,"updated":"2013-01-23 07:44:21.000000000","message":"I did not see the commit where this was introduced as part of the series for this blueprint.  I do not think we should make the statement that these drivers are deprecated for two reasons: \n\n1) unless I\u0027m missing something or there\u0027s some other mechanism that I have not yet seen by which the quantum api in nova will pass down a vif_type, deprecating everything but the LibvirtGenericVIFDriver amounts to REQUIRING that all quantum plugins implement what is now a voluntary API extension.  To require this would require making this extension part of the core API, which is not a decision that has even been proposed to my knowledge, much less agreed upon. \n\n2) As I mentioned on the mailing list, its still not clear to me how the new generic vif driver approach can handle heterogenous hypervisor types that require different vif_types or different bridge names.","commit_id":"0d0e50db4b78eb2be1b24a59043b806d07dd8a9a"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"68566bf92bb02a4338ce298b8e8e850eacddcf5f","unresolved":false,"context_lines":[{"line_number":346,"context_line":"        LOG.warn(_(\"LibvirtOpenVswitchDriver is deprecated and \""},{"line_number":347,"context_line":"                   \"will be removed in the Hxxxx release. Please \""},{"line_number":348,"context_line":"                   \"update the \u0027libvirt_vif_driver\u0027 config parameter \""},{"line_number":349,"context_line":"                   \"to use the LibvirtGenericVIFDriver class instead\"))"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def get_bridge_name(self, network):"},{"line_number":352,"context_line":"        return network.get(\u0027bridge\u0027) or CONF.libvirt_ovs_bridge"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAM3%2F%2Fatg%3D","line":349,"in_reply_to":"AAAAM3%2F%2Fa0Q%3D","updated":"2013-01-23 11:08:17.000000000","message":"My understanding was that all quantum plugins will provide the vif_type. As can be seen from the code changes in the 3 quantum plugins already ported to provide it, this is a trivial enhancement to Quantum. Having this be present in all Quantum plugins is absolutely fundamental to making Nova VIF configuration remotely sane, by removing all their hardcoded assumptions about Quantum implementation details which may break at any time Quantum code is changed. Of course not all plugins have been ported yet, hence why these stubbed VIF drivers remain - removal in the Hxxx release allows plenty of time for remaining plugins to be ported.","commit_id":"0d0e50db4b78eb2be1b24a59043b806d07dd8a9a"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"change_message_id":"34843e0b7cb85c1abfe7453230ee20fb8fcd447c","unresolved":false,"context_lines":[{"line_number":346,"context_line":"        LOG.warn(_(\"LibvirtOpenVswitchDriver is deprecated and \""},{"line_number":347,"context_line":"                   \"will be removed in the Hxxxx release. Please \""},{"line_number":348,"context_line":"                   \"update the \u0027libvirt_vif_driver\u0027 config parameter \""},{"line_number":349,"context_line":"                   \"to use the LibvirtGenericVIFDriver class instead\"))"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def get_bridge_name(self, network):"},{"line_number":352,"context_line":"        return network.get(\u0027bridge\u0027) or CONF.libvirt_ovs_bridge"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAM3%2F%2FZYo%3D","line":349,"in_reply_to":"AAAAM3%2F%2Fatg%3D","updated":"2013-01-24 05:07:54.000000000","message":"Let\u0027s chat about this on IRC.  My concern is certainly not that its too hard to make small code changes to Quantum.  My concern is that there are use cases that Quantum supports today that do not seem to be support by the new model.  Breaking use cases already in production for OpenStack seems like a very bad thing, so until I can understand how that is not the case, I\u0027m still against announcing deprecation.","commit_id":"0d0e50db4b78eb2be1b24a59043b806d07dd8a9a"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"change_message_id":"9e664e5137a2414bee1f0513828567248f024736","unresolved":false,"context_lines":[{"line_number":454,"context_line":"        LOG.warn(_(\"LibvirtOpenVswitchVirtualPortDriver is deprecated and \""},{"line_number":455,"context_line":"                   \"will be removed in the Hxxxx release. Please \""},{"line_number":456,"context_line":"                   \"update the \u0027libvirt_vif_driver\u0027 config parameter \""},{"line_number":457,"context_line":"                   \"to use the LibvirtGenericVIFDriver class instead\"))"},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    def get_bridge_name(self, network):"},{"line_number":460,"context_line":"        return network.get(\u0027bridge\u0027) or CONF.libvirt_ovs_bridge"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAM3%2F%2Fa0M%3D","line":457,"updated":"2013-01-23 07:44:21.000000000","message":"same comment as above.","commit_id":"0d0e50db4b78eb2be1b24a59043b806d07dd8a9a"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0eb923362ff6713f465d23d79e31cce86a60ba36","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, get_connection):"},{"line_number":58,"context_line":"        self.get_connection \u003d get_connection"},{"line_number":59,"context_line":"        self.libvirt_version \u003d None"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def has_libvirt_version(self, want):"},{"line_number":62,"context_line":"        if self.libvirt_version is None:"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAM3%2F%2FOZQ%3D","line":59,"updated":"2013-02-01 12:18:08.000000000","message":"Question: why not invoke the has_libvirt_version function here?\nYou can set a flag and then use the flag in get_config_ovs. \nIt will save function calls on the plug and unplug","commit_id":"c0013abb9996d129f880758466477d18dfe9cbe7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"ae7575085a3ce1cbc09dab4e176271050c0c13ef","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, get_connection):"},{"line_number":58,"context_line":"        self.get_connection \u003d get_connection"},{"line_number":59,"context_line":"        self.libvirt_version \u003d None"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def has_libvirt_version(self, want):"},{"line_number":62,"context_line":"        if self.libvirt_version is None:"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAM3%2F%2FIG0%3D","line":59,"in_reply_to":"AAAAM3%2F%2FODY%3D","updated":"2013-02-06 10:31:30.000000000","message":"This is just a pointless discussion. What is done here is fine - there is no measurable performance problem from doing a single method call here, given that the libvirt version is cached.","commit_id":"c0013abb9996d129f880758466477d18dfe9cbe7"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"fb634b943286833e98ec38ab199180854d4933ff","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, get_connection):"},{"line_number":58,"context_line":"        self.get_connection \u003d get_connection"},{"line_number":59,"context_line":"        self.libvirt_version \u003d None"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def has_libvirt_version(self, want):"},{"line_number":62,"context_line":"        if self.libvirt_version is None:"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAM3%2F%2FODY%3D","line":59,"in_reply_to":"AAAAM3%2F%2FOTU%3D","updated":"2013-02-01 17:03:01.000000000","message":"It could be mocked in the test suite.","commit_id":"c0013abb9996d129f880758466477d18dfe9cbe7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"d4e289a1f61077dc6b17a69e036cc5b5d4b4e829","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, get_connection):"},{"line_number":58,"context_line":"        self.get_connection \u003d get_connection"},{"line_number":59,"context_line":"        self.libvirt_version \u003d None"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def has_libvirt_version(self, want):"},{"line_number":62,"context_line":"        if self.libvirt_version is None:"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAM3%2F%2FOTU%3D","line":59,"in_reply_to":"AAAAM3%2F%2FOTc%3D","updated":"2013-02-01 14:04:37.000000000","message":"For sake of the test suite, the libvirt driver is structured to avoid initializing the connection during the constructor","commit_id":"c0013abb9996d129f880758466477d18dfe9cbe7"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"11c040e485a780b01ae7291d89c8b8294b3a40a9","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, get_connection):"},{"line_number":58,"context_line":"        self.get_connection \u003d get_connection"},{"line_number":59,"context_line":"        self.libvirt_version \u003d None"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def has_libvirt_version(self, want):"},{"line_number":62,"context_line":"        if self.libvirt_version is None:"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAM3%2F%2FOTc%3D","line":59,"in_reply_to":"AAAAM3%2F%2FOY4%3D","updated":"2013-02-01 14:02:07.000000000","message":"So at this stage self.get_connection() will not work?","commit_id":"c0013abb9996d129f880758466477d18dfe9cbe7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"8da5b5b454856b2c66b5f11a99922dc9f8e4b402","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, get_connection):"},{"line_number":58,"context_line":"        self.get_connection \u003d get_connection"},{"line_number":59,"context_line":"        self.libvirt_version \u003d None"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def has_libvirt_version(self, want):"},{"line_number":62,"context_line":"        if self.libvirt_version is None:"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAM3%2F%2FOY4%3D","line":59,"in_reply_to":"AAAAM3%2F%2FOZQ%3D","updated":"2013-02-01 12:21:05.000000000","message":"We don\u0027t have any connection object available at the time the constructor runs - that\u0027s why we pass in a \u0027get_connection\u0027 helper, rather than an connection instance itself.","commit_id":"c0013abb9996d129f880758466477d18dfe9cbe7"}]}