)]}'
{"nova/virt/libvirt/driver.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"0a058271ab99cfc3a72eb388db099e7e8b968fcb","unresolved":false,"context_lines":[{"line_number":3241,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3242,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3243,"context_line":""},{"line_number":3244,"context_line":"        if not utils.is_neutron():"},{"line_number":3245,"context_line":"            try:"},{"line_number":3246,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3247,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9ozM%3D","line":3244,"updated":"2013-12-09 13:21:24.000000000","message":"This looks really dubious to me. At least one of the Neutron deployment modes is supposed to be functionally equivalent to nova-network, such that people will be able to do a in-place upgrade from nova-network to neutron. Since nova-network wants hairpin mode enabled, this implies that there will be at least one neutron deployment mode which requires it enabled too, otherwise live upgrade won\u0027t work.\n\nThis in turn says to me that disabling / enabling hairpin mode must be a per-VIF option passed as a config param in the nova.network.model.VIF class and processed by the vif.py code.\n\nSecond this is the VM startup code path, but what about the VIF hotplug codepath. The same setup must be made in both places, which again says vif.py should handle this.","commit_id":"a9ba7c65be730c9b220823dc1b869ef6ac1893ee"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"e35038c06c012b923c3022ce5463e01b52a8a3a2","unresolved":false,"context_lines":[{"line_number":3241,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3242,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3243,"context_line":""},{"line_number":3244,"context_line":"        if not utils.is_neutron():"},{"line_number":3245,"context_line":"            try:"},{"line_number":3246,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3247,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2F6JE%3D","line":3244,"in_reply_to":"AAAATn%2F9frg%3D","updated":"2014-01-06 23:05:25.000000000","message":"There\u0027s something very fishy here - specifically, this hairpinning is only ever done in libvirt, never elsewhere.  Also, if you look in enable_hairpin, you\u0027ll notice that errors are ignored - because specific VIF driver types don\u0027t actually support hairpin, so where we can\u0027t enable it we just ignore the error caused by the attempt.\n\nAlso, no Neutron plugin currently in existence seems to require it, which is probably just as well since they\u0027re only going to get it for very specific libvirt configurations.  \n\nThat being the case, I think this *is* the right solution.  Putting in a complex flag-passing system to maintain behaviour that nothing uses is makework.","commit_id":"a9ba7c65be730c9b220823dc1b869ef6ac1893ee"},{"author":{"_account_id":4656,"name":"Sean M. Collins","email":"sean@coreitpro.com","username":"scollins"},"change_message_id":"35f9513433ebaac0c300f4a7146ba31a8ad138fe","unresolved":false,"context_lines":[{"line_number":3241,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3242,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3243,"context_line":""},{"line_number":3244,"context_line":"        if not utils.is_neutron():"},{"line_number":3245,"context_line":"            try:"},{"line_number":3246,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3247,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9frg%3D","line":3244,"in_reply_to":"AAAATn%2F9jnM%3D","updated":"2013-12-10 22:39:29.000000000","message":"If it were to be a per-VIF parameter, the default should be to disable it, and only in the unusual case should it be activated, since this currently breaks IPv6 on guest instances. So this patch would still be valid - I would need to add a second patch to this topic to look for a VIF parameter and turn it on in those cases.\n\nThis is at least my belief.","commit_id":"a9ba7c65be730c9b220823dc1b869ef6ac1893ee"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"2d224ae1424cdff00910fbf48bfd1dd36c8b2fa4","unresolved":false,"context_lines":[{"line_number":3241,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3242,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3243,"context_line":""},{"line_number":3244,"context_line":"        if not utils.is_neutron():"},{"line_number":3245,"context_line":"            try:"},{"line_number":3246,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3247,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9jnM%3D","line":3244,"in_reply_to":"AAAATn%2F9lzA%3D","updated":"2013-12-10 09:37:21.000000000","message":"I\u0027m not going to debate the merits of different impls, but the point is that this should be a per-VIF parameter not a global toggle is_neutron toggle, so that it can cope with potentially differing requirements of different neutron plugins.","commit_id":"a9ba7c65be730c9b220823dc1b869ef6ac1893ee"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"05a24b14c3a7808c3472e4056d1011186b189589","unresolved":false,"context_lines":[{"line_number":3241,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3242,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3243,"context_line":""},{"line_number":3244,"context_line":"        if not utils.is_neutron():"},{"line_number":3245,"context_line":"            try:"},{"line_number":3246,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3247,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9lzA%3D","line":3244,"in_reply_to":"AAAATn%2F9ozM%3D","updated":"2013-12-09 21:49:34.000000000","message":"The hairpin in nova-network is required because the rewrites of fixed to floating IP exist between the VNIC and the bridge it\u0027s plugged into, as rewrite rules - iirc this is so that you could ping your own floating IP.  You can argue all you like about multihost, but I guarantee no-one in Neutron is going back there - those rewrite rules are re-implemeted in the switch as NATting behaviour.  If you want the same behaviour (and I recommend we write a test for it) the hairpin would want to be on the opposite side of the router, not here.","commit_id":"a9ba7c65be730c9b220823dc1b869ef6ac1893ee"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df6cc61d99f2bffe3a9c6c1cad8a46da2fbdcdc5","unresolved":false,"context_lines":[{"line_number":3241,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3242,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3243,"context_line":""},{"line_number":3244,"context_line":"        if not utils.is_neutron():"},{"line_number":3245,"context_line":"            try:"},{"line_number":3246,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3247,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2F1mw%3D","line":3244,"in_reply_to":"AAAAUH%2F%2F1ok%3D","updated":"2014-01-07 21:23:31.000000000","message":"Got it. Thanks Ian. :)","commit_id":"a9ba7c65be730c9b220823dc1b869ef6ac1893ee"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"bf32d2f4d1e6659e3ff7d3746e6bc420d76bf5be","unresolved":false,"context_lines":[{"line_number":3241,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3242,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3243,"context_line":""},{"line_number":3244,"context_line":"        if not utils.is_neutron():"},{"line_number":3245,"context_line":"            try:"},{"line_number":3246,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3247,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2F1ok%3D","line":3244,"in_reply_to":"AAAAUH%2F%2F2Wg%3D","updated":"2014-01-07 21:19:26.000000000","message":"Hairpinning would not be enabled if enable_hairpin was not called (it\u0027s off by default, this action has to be here to enable it).","commit_id":"a9ba7c65be730c9b220823dc1b869ef6ac1893ee"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"233db8c3370e439e9e4c4604be37bd8156f52a87","unresolved":false,"context_lines":[{"line_number":3241,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3242,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3243,"context_line":""},{"line_number":3244,"context_line":"        if not utils.is_neutron():"},{"line_number":3245,"context_line":"            try:"},{"line_number":3246,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3247,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2F2Wg%3D","line":3244,"in_reply_to":"AAAAUH%2F%2F6JE%3D","updated":"2014-01-07 17:44:39.000000000","message":"So what happens when nova-network is removed (supposed to be happening soon, yes?) Is this code block supposed to then be entirely removed or should it be replaced with a call to a self._disable_hairpin(), since this seems to indicate that if Neutron is used at all, then hairpinning should never be enabled.","commit_id":"a9ba7c65be730c9b220823dc1b869ef6ac1893ee"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"161fbcd103251015f7f0a08239954d81a1473379","unresolved":false,"context_lines":[{"line_number":3370,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3371,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3372,"context_line":""},{"line_number":3373,"context_line":"        if not utils.is_neutron():"},{"line_number":3374,"context_line":"            try:"},{"line_number":3375,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3376,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUH%2F%2FXzY%3D","line":3373,"updated":"2014-01-14 14:31:24.000000000","message":"As raised before, this is just plain wrong.\n\nWe have designed the nova.network.model.VIF \u0026 nova.virt.libvirt.vif.* classes explicitly such that we don\u0027t end up littering the code with network impl specific conditionals like \u0027is_neutron()\u0027. Please make proper use of the VIF abstractions we have rather than just hacking it in an adhoc manner.","commit_id":"dc839130f47230d50dbc0dd3f30521c8b5ef2bdf"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"7817a857b648935d2513ec13185fe2dfbc2487dc","unresolved":false,"context_lines":[{"line_number":3370,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3371,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3372,"context_line":""},{"line_number":3373,"context_line":"        if not utils.is_neutron():"},{"line_number":3374,"context_line":"            try:"},{"line_number":3375,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3376,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUH%2F%2FXJ4%3D","line":3373,"in_reply_to":"AAAAUH%2F%2FXis%3D","updated":"2014-01-14 16:47:04.000000000","message":"So I tried to actually implement what I suggested. We can\u0027t just set hairpin in the plug() method since we can\u0027t guarantee the VIF exists at that point. So I tried to add a postrun() method to be invoke once the VM is running. Unfortunately there are some codepaths where network model is not available :-( So my suggestion won\u0027t work after all. As such I\u0027ll reluctantly remove my -1 here.","commit_id":"dc839130f47230d50dbc0dd3f30521c8b5ef2bdf"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"52d9e2ba6a416647372cd0597fca6dc2c7abddde","unresolved":false,"context_lines":[{"line_number":3370,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3371,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3372,"context_line":""},{"line_number":3373,"context_line":"        if not utils.is_neutron():"},{"line_number":3374,"context_line":"            try:"},{"line_number":3375,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3376,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUH%2F%2FXis%3D","line":3373,"in_reply_to":"AAAAUH%2F%2FXjA%3D","updated":"2014-01-14 15:28:10.000000000","message":"Particularly when enable hairpin doesn\u0027t always...","commit_id":"dc839130f47230d50dbc0dd3f30521c8b5ef2bdf"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"55ea021f030da119f180e769a4a43c0bc79559ab","unresolved":false,"context_lines":[{"line_number":3370,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3371,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3372,"context_line":""},{"line_number":3373,"context_line":"        if not utils.is_neutron():"},{"line_number":3374,"context_line":"            try:"},{"line_number":3375,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3376,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUH%2F%2FXjA%3D","line":3373,"in_reply_to":"AAAAUH%2F%2FXnI%3D","updated":"2014-01-14 15:27:18.000000000","message":"I see your point, but actually in this instance by having this functionality - unconditionally - you\u0027re behaving differently to every other hypervisor, probably in a way that\u0027s detectably so. \n\nThe question is, if we make this a property of the model or the VIF plugging class then what should be setting it enabled for nova-network?  And how do you justify doing it that way and then leaving the other hypervisors ignoring that flag?  There\u0027s no really good answer here.","commit_id":"dc839130f47230d50dbc0dd3f30521c8b5ef2bdf"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"6ab12253b6610a8a4ec4f6628d05682e46a45907","unresolved":false,"context_lines":[{"line_number":3370,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3371,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3372,"context_line":""},{"line_number":3373,"context_line":"        if not utils.is_neutron():"},{"line_number":3374,"context_line":"            try:"},{"line_number":3375,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3376,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUH%2F%2FXnI%3D","line":3373,"in_reply_to":"AAAAUH%2F%2FXtY%3D","updated":"2014-01-14 15:13:58.000000000","message":"I understand what you\u0027re saying and don\u0027t disagree with it, but that is not what I was complaining about here.  We have a design with the VIF classes that avoids us having to stuff  \"if is_neutron/is_nova_network\" in our driver.py code. I don\u0027t see any reason why we should ignore that and start adding  is_neutron or is_nova_network to our code here.","commit_id":"dc839130f47230d50dbc0dd3f30521c8b5ef2bdf"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"8fa3904f0be1d15b23dd2dad8b7594286e0acc47","unresolved":false,"context_lines":[{"line_number":3370,"context_line":"                                \"defined domain with xml: %s\") %"},{"line_number":3371,"context_line":"                              domain.XMLDesc(0))"},{"line_number":3372,"context_line":""},{"line_number":3373,"context_line":"        if not utils.is_neutron():"},{"line_number":3374,"context_line":"            try:"},{"line_number":3375,"context_line":"                self._enable_hairpin(domain.XMLDesc(0))"},{"line_number":3376,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUH%2F%2FXtY%3D","line":3373,"in_reply_to":"AAAAUH%2F%2FXzY%3D","updated":"2014-01-14 14:46:55.000000000","message":"Daniel, please see my email on the mailing list from last week giving extensive reasoning why this is, in fact, correct.  \n\n\u003crant\u003e\nIn nova-network, yes, hairpinning is required because nova-network puts all its NAT rules on the port rather than on ingress.  Hairpinning is a not-great solution to one implication of this.  Neutron does them on ingress (in routers, in fact), not on the port, and hairpinning is (a) not required and (b) completely undesirable.  The same hairpinning is specific to libvirt (no-one changed any of the other virt drivers) and doesn\u0027t work universally for libvirt (because some of the VIF plug types attach to an OVS which doesn\u0027t support hairpinning - where enable_hairpin is designed to silently fail).\n\nIf we follow that argument to its conclusion the only thing you could reasonably say about this code is that it should actually read \u0027if utils.is_nova_network():\u0027.\n\u003c/rant\u003e and with apologies.","commit_id":"dc839130f47230d50dbc0dd3f30521c8b5ef2bdf"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"265b4ccf61cf2b4f448eaf7841ff96e0d428a86e","unresolved":false,"context_lines":[{"line_number":252,"context_line":"    cfg.BoolOpt(\u0027use_hairpin\u0027,"},{"line_number":253,"context_line":"                 default\u003dTrue,"},{"line_number":254,"context_line":"                 help\u003d\u0027Enable hairpinning on the bridge interface \u0027"},{"line_number":255,"context_line":"                      \u0027which allows an instance to ping its own floating-ip \u0027"},{"line_number":256,"context_line":"                      \u0027NOTE: This breaks IPv6\u0027),"},{"line_number":257,"context_line":"    ]"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAUH%2F%2BQzc%3D","line":255,"updated":"2014-01-28 15:38:33.000000000","message":"floating-ip \u003d\u003e floating-ip. \nMissing a full stop","commit_id":"af7402da1888b90fe61e164754114dacb68566db"}],"nova/virt/libvirt/vif.py":[{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"ba5f0d7a0eb65d5062394565178e7eb5b0e509f5","unresolved":false,"context_lines":[{"line_number":452,"context_line":"                                          instance[\u0027uuid\u0027])"},{"line_number":453,"context_line":"        if CONF.use_ipv6:"},{"line_number":454,"context_line":"            utils.execute(\u0027brctl\u0027, br_name, v1_name,"},{"line_number":455,"context_line":"                          \u0027hairpin\u0027, \u0027off\u0027, run_as_root\u003dTrue)"},{"line_number":456,"context_line":""},{"line_number":457,"context_line":"    def plug_ovs(self, instance, vif):"},{"line_number":458,"context_line":"        if self.get_firewall_required():"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BnfI%3D","line":455,"updated":"2013-11-22 12:24:21.000000000","message":"If you search the code you\u0027ll also find the code that turns it on - perhaps better not to turn it on at all, rather than to turn it on and then off again.","commit_id":"30b95afdab78f710393ff7bda90b6be7cac4edca"}]}